Re: [PATCH v1 1/3] Add map/unmap ioctl and clean mappings post-guest

[Christian Borntraeger]

Am 09.03.26 um 18:12 schrieb Douglas Freimuth:
> On 3/9/26 5:27 AM, Christian Borntraeger wrote:
> > Am 08.03.26 um 04:04 schrieb Douglas Freimuth:
> > > Fencing of Fast Inject in Secure Execution environments is enabled in
> > > this patch by not mapping adapter indicator pages. In Secure Execution
> > [...]
> >
> > > @@ -2477,14 +2572,28 @@ static int modify_io_adapter(struct kvm_device *dev,
> > >           if (ret > 0)
> > >               ret = 0;
> > >           break;
> > > -    /*
> > > -     * The following operations are no longer needed and therefore no-ops.
> > > -     * The gpa to hva translation is done when an IRQ route is set up. The
> > > -     * set_irq code uses get_user_pages_remote() to do the actual write.
> > > -     */
> > >       case KVM_S390_IO_ADAPTER_MAP:
> > >       case KVM_S390_IO_ADAPTER_UNMAP:
> > > -        ret = 0;
> > > +        mutex_lock(&dev->kvm->lock);
> > > +        if (kvm_s390_pv_is_protected(dev->kvm)) {
> > > +            mutex_unlock(&dev->kvm->lock);
> > > +            break;
> > > +        }
> >
> >
> > I guess this works for a well behaving userspaces, but a bad QEMU could in theory
> > not do the unmap on switch to secure.
> > Shall we maybe do -EINVAL on KVM_PV_ENABLE if there are still mapping left, or
> > to make it easier for userspace remove the old ADAPTER maps?
>
> Christian, thank you for your input. For this scenario, I will look into adding/testing removing the old adapter maps. I will start in kvm_s390_handle_pv() for CASE KVM_PV_ENABLE and I will essentially use most of the functionality in kvm_s390_destroy_adapters() where the maps are deleted if they exist.

Right, maybe just add a unmap_all_adapters function and call that.

> Discussion: During development and test I realized it appears a guest can only change state between non-SE and SE during a reboot. Thus the unmap and map is called which hits the fencing in the current patch. Additionally, a more draconian fencing could possibly be done if needed, by checking for the existence of SE firmware in the CMDLINE and prevent any mapping from occurring on those systems that support SE.

Yes, diagnose 308 code 10 switches in a reboot like style to secure and diagnose 308 code 3 or 4 switch back to non secure.