diff mbox series

[v5,7/9] s390x/pci: enable adapter event notification for interpreted devices

Message ID 20220404181726.60291-8-mjrosato@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series s390x/pci: zPCI interpretation support | expand

Commit Message

Matthew Rosato April 4, 2022, 6:17 p.m. UTC
Use the associated kvm ioctl operation to enable adapter event notification
and forwarding for devices when requested.  This feature will be set up
with or without firmware assist based upon the 'forwarding_assist' setting.

Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
---
 hw/s390x/s390-pci-bus.c         | 20 ++++++++++++++---
 hw/s390x/s390-pci-inst.c        | 40 +++++++++++++++++++++++++++++++--
 hw/s390x/s390-pci-kvm.c         | 30 +++++++++++++++++++++++++
 include/hw/s390x/s390-pci-bus.h |  1 +
 include/hw/s390x/s390-pci-kvm.h | 14 ++++++++++++
 5 files changed, 100 insertions(+), 5 deletions(-)

Comments

Pierre Morel April 22, 2022, 9:39 a.m. UTC | #1
On 4/4/22 20:17, Matthew Rosato wrote:
> Use the associated kvm ioctl operation to enable adapter event notification
> and forwarding for devices when requested.  This feature will be set up
> with or without firmware assist based upon the 'forwarding_assist' setting.
> 
> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
> ---
>   hw/s390x/s390-pci-bus.c         | 20 ++++++++++++++---
>   hw/s390x/s390-pci-inst.c        | 40 +++++++++++++++++++++++++++++++--
>   hw/s390x/s390-pci-kvm.c         | 30 +++++++++++++++++++++++++
>   include/hw/s390x/s390-pci-bus.h |  1 +
>   include/hw/s390x/s390-pci-kvm.h | 14 ++++++++++++
>   5 files changed, 100 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 9c02d31250..47918d2ce9 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -190,7 +190,10 @@ void s390_pci_sclp_deconfigure(SCCB *sccb)
>           rc = SCLP_RC_NO_ACTION_REQUIRED;
>           break;
>       default:
> -        if (pbdev->summary_ind) {
> +        if (pbdev->interp && (pbdev->fh & FH_MASK_ENABLE)) {
> +            /* Interpreted devices were using interrupt forwarding */
> +            s390_pci_kvm_aif_disable(pbdev);

Same remark as for the kernel part.
The VFIO device is already initialized and the action is on this device, 
Shouldn't we use the VFIO device interface instead of the KVM interface?


regards,
Pierre
Matthew Rosato April 22, 2022, 12:10 p.m. UTC | #2
On 4/22/22 5:39 AM, Pierre Morel wrote:
> 
> 
> On 4/4/22 20:17, Matthew Rosato wrote:
>> Use the associated kvm ioctl operation to enable adapter event 
>> notification
>> and forwarding for devices when requested.  This feature will be set up
>> with or without firmware assist based upon the 'forwarding_assist' 
>> setting.
>>
>> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
>> ---
>>   hw/s390x/s390-pci-bus.c         | 20 ++++++++++++++---
>>   hw/s390x/s390-pci-inst.c        | 40 +++++++++++++++++++++++++++++++--
>>   hw/s390x/s390-pci-kvm.c         | 30 +++++++++++++++++++++++++
>>   include/hw/s390x/s390-pci-bus.h |  1 +
>>   include/hw/s390x/s390-pci-kvm.h | 14 ++++++++++++
>>   5 files changed, 100 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
>> index 9c02d31250..47918d2ce9 100644
>> --- a/hw/s390x/s390-pci-bus.c
>> +++ b/hw/s390x/s390-pci-bus.c
>> @@ -190,7 +190,10 @@ void s390_pci_sclp_deconfigure(SCCB *sccb)
>>           rc = SCLP_RC_NO_ACTION_REQUIRED;
>>           break;
>>       default:
>> -        if (pbdev->summary_ind) {
>> +        if (pbdev->interp && (pbdev->fh & FH_MASK_ENABLE)) {
>> +            /* Interpreted devices were using interrupt forwarding */
>> +            s390_pci_kvm_aif_disable(pbdev);
> 
> Same remark as for the kernel part.
> The VFIO device is already initialized and the action is on this device, 
> Shouldn't we use the VFIO device interface instead of the KVM interface?
> 

I don't necessarily disagree, but in v3 of the kernel series I was told 
not to use VFIO ioctls to accomplish tasks that are unique to KVM (e.g. 
AEN interpretation) and to instead use a KVM ioctl.

VFIO_DEVICE_SET_IRQS won't work as-is for reasons described in the 
kernel series (e.g. we don't see any of the config space notifiers 
because of instruction interpretation) -- as far as I can figure we 
could add our own s390 code to QEMU to issue VFIO_DEVICE_SET_IRQS 
directly for an interpreted device, but I think would also need 
s390-specific changes to VFIO_DEVICE_SET_IRQS accommodate this (e.g. 
maybe something like a VFIO_IRQ_SET_DATA_S390AEN where we can then 
specify the aen information in vfio_irq_set.data -- or something else I 
haven't though of yet) -- I can try to look at this some more and see if 
I get a good idea.
Pierre Morel May 2, 2022, 7:48 a.m. UTC | #3
On 4/22/22 14:10, Matthew Rosato wrote:
> On 4/22/22 5:39 AM, Pierre Morel wrote:
>>
>>
>> On 4/4/22 20:17, Matthew Rosato wrote:
>>> Use the associated kvm ioctl operation to enable adapter event 
>>> notification
>>> and forwarding for devices when requested.  This feature will be set up
>>> with or without firmware assist based upon the 'forwarding_assist' 
>>> setting.
>>>
>>> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
>>> ---
>>>   hw/s390x/s390-pci-bus.c         | 20 ++++++++++++++---
>>>   hw/s390x/s390-pci-inst.c        | 40 +++++++++++++++++++++++++++++++--
>>>   hw/s390x/s390-pci-kvm.c         | 30 +++++++++++++++++++++++++
>>>   include/hw/s390x/s390-pci-bus.h |  1 +
>>>   include/hw/s390x/s390-pci-kvm.h | 14 ++++++++++++
>>>   5 files changed, 100 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
>>> index 9c02d31250..47918d2ce9 100644
>>> --- a/hw/s390x/s390-pci-bus.c
>>> +++ b/hw/s390x/s390-pci-bus.c
>>> @@ -190,7 +190,10 @@ void s390_pci_sclp_deconfigure(SCCB *sccb)
>>>           rc = SCLP_RC_NO_ACTION_REQUIRED;
>>>           break;
>>>       default:
>>> -        if (pbdev->summary_ind) {
>>> +        if (pbdev->interp && (pbdev->fh & FH_MASK_ENABLE)) {
>>> +            /* Interpreted devices were using interrupt forwarding */
>>> +            s390_pci_kvm_aif_disable(pbdev);
>>
>> Same remark as for the kernel part.
>> The VFIO device is already initialized and the action is on this 
>> device, Shouldn't we use the VFIO device interface instead of the KVM 
>> interface?
>>
> 
> I don't necessarily disagree, but in v3 of the kernel series I was told 
> not to use VFIO ioctls to accomplish tasks that are unique to KVM (e.g. 
> AEN interpretation) and to instead use a KVM ioctl.
> 
> VFIO_DEVICE_SET_IRQS won't work as-is for reasons described in the 
> kernel series (e.g. we don't see any of the config space notifiers 
> because of instruction interpretation) -- as far as I can figure we 
> could add our own s390 code to QEMU to issue VFIO_DEVICE_SET_IRQS 
> directly for an interpreted device, but I think would also need 
> s390-specific changes to VFIO_DEVICE_SET_IRQS accommodate this (e.g. 
> maybe something like a VFIO_IRQ_SET_DATA_S390AEN where we can then 
> specify the aen information in vfio_irq_set.data -- or something else I 

Hi,

yes this in VFIO_DEVICE_SET_IRQS is what I think should be done.

> haven't though of yet) -- I can try to look at this some more and see if 
> I get a good idea.



I understood that the demand was concerning the IOMMU but I may be wrong.
For my opinion, the handling of AEN is not specific to KVM but specific 
to the device, for example the code should be the same if Z ever decide 
to use XEN or another hypervizor, except for the GISA part but this part 
is already implemented in KVM in a way it can be used from a device like 
in VFIO AP.

@Alex, what do you think?

Regards,
Pierre
Niklas Schnelle May 2, 2022, 9:19 a.m. UTC | #4
On Mon, 2022-05-02 at 09:48 +0200, Pierre Morel wrote:
> 
> On 4/22/22 14:10, Matthew Rosato wrote:
> > On 4/22/22 5:39 AM, Pierre Morel wrote:
> > > 
> > > On 4/4/22 20:17, Matthew Rosato wrote:
> > > > Use the associated kvm ioctl operation to enable adapter event 
> > > > notification
> > > > and forwarding for devices when requested.  This feature will be set up
> > > > with or without firmware assist based upon the 'forwarding_assist' 
> > > > setting.
> > > > 
> > > > Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
> > > > ---
> > > >   hw/s390x/s390-pci-bus.c         | 20 ++++++++++++++---
> > > >   hw/s390x/s390-pci-inst.c        | 40 +++++++++++++++++++++++++++++++--
> > > >   hw/s390x/s390-pci-kvm.c         | 30 +++++++++++++++++++++++++
> > > >   include/hw/s390x/s390-pci-bus.h |  1 +
> > > >   include/hw/s390x/s390-pci-kvm.h | 14 ++++++++++++
> > > >   5 files changed, 100 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> > > > index 9c02d31250..47918d2ce9 100644
> > > > --- a/hw/s390x/s390-pci-bus.c
> > > > +++ b/hw/s390x/s390-pci-bus.c
> > > > @@ -190,7 +190,10 @@ void s390_pci_sclp_deconfigure(SCCB *sccb)
> > > >           rc = SCLP_RC_NO_ACTION_REQUIRED;
> > > >           break;
> > > >       default:
> > > > -        if (pbdev->summary_ind) {
> > > > +        if (pbdev->interp && (pbdev->fh & FH_MASK_ENABLE)) {
> > > > +            /* Interpreted devices were using interrupt forwarding */
> > > > +            s390_pci_kvm_aif_disable(pbdev);
> > > 
> > > Same remark as for the kernel part.
> > > The VFIO device is already initialized and the action is on this 
> > > device, Shouldn't we use the VFIO device interface instead of the KVM 
> > > interface?
> > > 
> > 
> > I don't necessarily disagree, but in v3 of the kernel series I was told 
> > not to use VFIO ioctls to accomplish tasks that are unique to KVM (e.g. 
> > AEN interpretation) and to instead use a KVM ioctl.
> > 
> > VFIO_DEVICE_SET_IRQS won't work as-is for reasons described in the 
> > kernel series (e.g. we don't see any of the config space notifiers 
> > because of instruction interpretation) -- as far as I can figure we 
> > could add our own s390 code to QEMU to issue VFIO_DEVICE_SET_IRQS 
> > directly for an interpreted device, but I think would also need 
> > s390-specific changes to VFIO_DEVICE_SET_IRQS accommodate this (e.g. 
> > maybe something like a VFIO_IRQ_SET_DATA_S390AEN where we can then 
> > specify the aen information in vfio_irq_set.data -- or something else I 
> 
> Hi,
> 
> yes this in VFIO_DEVICE_SET_IRQS is what I think should be done.
> 
> > haven't though of yet) -- I can try to look at this some more and see if 
> > I get a good idea.
> 
> 
> I understood that the demand was concerning the IOMMU but I may be wrong.
> For my opinion, the handling of AEN is not specific to KVM but specific 
> to the device, for example the code should be the same if Z ever decide 
> to use XEN or another hypervizor, except for the GISA part but this part 
> is already implemented in KVM in a way it can be used from a device like 
> in VFIO AP.
> 
> @Alex, what do you think?
> 
> Regards,
> Pierre
> 

As I understand it the question isn't if it is specific to KVM but
rather if it is specific to virtualization. As vfio-pci is also used
for non virtualization purposes such as with DPDK/SPDK or a fully
emulating QEMU, it should only be in VFIO if it is relevant for these
kinds of user-space PCI accesses too. I'm not an AEN expert but as I
understand it, this does forwarding interrupts into a SIE context which
only makes sense for virtualization not for general user-space PCI.
Pierre Morel May 2, 2022, 11:30 a.m. UTC | #5
On 5/2/22 11:19, Niklas Schnelle wrote:
> On Mon, 2022-05-02 at 09:48 +0200, Pierre Morel wrote:
>>
>> On 4/22/22 14:10, Matthew Rosato wrote:
>>> On 4/22/22 5:39 AM, Pierre Morel wrote:
>>>>
>>>> On 4/4/22 20:17, Matthew Rosato wrote:
>>>>> Use the associated kvm ioctl operation to enable adapter event
>>>>> notification
>>>>> and forwarding for devices when requested.  This feature will be set up
>>>>> with or without firmware assist based upon the 'forwarding_assist'
>>>>> setting.
>>>>>
>>>>> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
>>>>> ---
>>>>>    hw/s390x/s390-pci-bus.c         | 20 ++++++++++++++---
>>>>>    hw/s390x/s390-pci-inst.c        | 40 +++++++++++++++++++++++++++++++--
>>>>>    hw/s390x/s390-pci-kvm.c         | 30 +++++++++++++++++++++++++
>>>>>    include/hw/s390x/s390-pci-bus.h |  1 +
>>>>>    include/hw/s390x/s390-pci-kvm.h | 14 ++++++++++++
>>>>>    5 files changed, 100 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
>>>>> index 9c02d31250..47918d2ce9 100644
>>>>> --- a/hw/s390x/s390-pci-bus.c
>>>>> +++ b/hw/s390x/s390-pci-bus.c
>>>>> @@ -190,7 +190,10 @@ void s390_pci_sclp_deconfigure(SCCB *sccb)
>>>>>            rc = SCLP_RC_NO_ACTION_REQUIRED;
>>>>>            break;
>>>>>        default:
>>>>> -        if (pbdev->summary_ind) {
>>>>> +        if (pbdev->interp && (pbdev->fh & FH_MASK_ENABLE)) {
>>>>> +            /* Interpreted devices were using interrupt forwarding */
>>>>> +            s390_pci_kvm_aif_disable(pbdev);
>>>>
>>>> Same remark as for the kernel part.
>>>> The VFIO device is already initialized and the action is on this
>>>> device, Shouldn't we use the VFIO device interface instead of the KVM
>>>> interface?
>>>>
>>>
>>> I don't necessarily disagree, but in v3 of the kernel series I was told
>>> not to use VFIO ioctls to accomplish tasks that are unique to KVM (e.g.
>>> AEN interpretation) and to instead use a KVM ioctl.
>>>
>>> VFIO_DEVICE_SET_IRQS won't work as-is for reasons described in the
>>> kernel series (e.g. we don't see any of the config space notifiers
>>> because of instruction interpretation) -- as far as I can figure we
>>> could add our own s390 code to QEMU to issue VFIO_DEVICE_SET_IRQS
>>> directly for an interpreted device, but I think would also need
>>> s390-specific changes to VFIO_DEVICE_SET_IRQS accommodate this (e.g.
>>> maybe something like a VFIO_IRQ_SET_DATA_S390AEN where we can then
>>> specify the aen information in vfio_irq_set.data -- or something else I
>>
>> Hi,
>>
>> yes this in VFIO_DEVICE_SET_IRQS is what I think should be done.
>>
>>> haven't though of yet) -- I can try to look at this some more and see if
>>> I get a good idea.
>>
>>
>> I understood that the demand was concerning the IOMMU but I may be wrong.
>> For my opinion, the handling of AEN is not specific to KVM but specific
>> to the device, for example the code should be the same if Z ever decide
>> to use XEN or another hypervizor, except for the GISA part but this part
>> is already implemented in KVM in a way it can be used from a device like
>> in VFIO AP.
>>
>> @Alex, what do you think?
>>
>> Regards,
>> Pierre
>>
> 
> As I understand it the question isn't if it is specific to KVM but
> rather if it is specific to virtualization. As vfio-pci is also used
> for non virtualization purposes such as with DPDK/SPDK or a fully
> emulating QEMU, it should only be in VFIO if it is relevant for these
> kinds of user-space PCI accesses too. I'm not an AEN expert but as I
> understand it, this does forwarding interrupts into a SIE context which
> only makes sense for virtualization not for general user-space PCI.
> 

Being in VFIO kernel part does not mean that this part should be called 
from any user of VFIO in userland.
That is a reason why I did propose an extension and not using the 
current implementation of VFIO_DEVICE_SET_IRQS as is.

The reason behind is that the AEN hardware handling is device specific: 
we need the Function Handle to program AEN.

If the API is through KVM which is device agnostic the implementation in 
KVM has to search through the system to find the device being handled to 
apply AEN on it.

This not the logical way for me and it is a potential source of problems 
for future extensions.

Regards,
Pierre
Matthew Rosato May 2, 2022, 7:57 p.m. UTC | #6
On 5/2/22 7:30 AM, Pierre Morel wrote:
> 
> 
> On 5/2/22 11:19, Niklas Schnelle wrote:
>> On Mon, 2022-05-02 at 09:48 +0200, Pierre Morel wrote:
>>>
>>> On 4/22/22 14:10, Matthew Rosato wrote:
>>>> On 4/22/22 5:39 AM, Pierre Morel wrote:
>>>>>
>>>>> On 4/4/22 20:17, Matthew Rosato wrote:
>>>>>> Use the associated kvm ioctl operation to enable adapter event
>>>>>> notification
>>>>>> and forwarding for devices when requested.  This feature will be 
>>>>>> set up
>>>>>> with or without firmware assist based upon the 'forwarding_assist'
>>>>>> setting.
>>>>>>
>>>>>> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
>>>>>> ---
>>>>>>    hw/s390x/s390-pci-bus.c         | 20 ++++++++++++++---
>>>>>>    hw/s390x/s390-pci-inst.c        | 40 
>>>>>> +++++++++++++++++++++++++++++++--
>>>>>>    hw/s390x/s390-pci-kvm.c         | 30 +++++++++++++++++++++++++
>>>>>>    include/hw/s390x/s390-pci-bus.h |  1 +
>>>>>>    include/hw/s390x/s390-pci-kvm.h | 14 ++++++++++++
>>>>>>    5 files changed, 100 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
>>>>>> index 9c02d31250..47918d2ce9 100644
>>>>>> --- a/hw/s390x/s390-pci-bus.c
>>>>>> +++ b/hw/s390x/s390-pci-bus.c
>>>>>> @@ -190,7 +190,10 @@ void s390_pci_sclp_deconfigure(SCCB *sccb)
>>>>>>            rc = SCLP_RC_NO_ACTION_REQUIRED;
>>>>>>            break;
>>>>>>        default:
>>>>>> -        if (pbdev->summary_ind) {
>>>>>> +        if (pbdev->interp && (pbdev->fh & FH_MASK_ENABLE)) {
>>>>>> +            /* Interpreted devices were using interrupt 
>>>>>> forwarding */
>>>>>> +            s390_pci_kvm_aif_disable(pbdev);
>>>>>
>>>>> Same remark as for the kernel part.
>>>>> The VFIO device is already initialized and the action is on this
>>>>> device, Shouldn't we use the VFIO device interface instead of the KVM
>>>>> interface?
>>>>>
>>>>
>>>> I don't necessarily disagree, but in v3 of the kernel series I was told
>>>> not to use VFIO ioctls to accomplish tasks that are unique to KVM (e.g.
>>>> AEN interpretation) and to instead use a KVM ioctl.
>>>>
>>>> VFIO_DEVICE_SET_IRQS won't work as-is for reasons described in the
>>>> kernel series (e.g. we don't see any of the config space notifiers
>>>> because of instruction interpretation) -- as far as I can figure we
>>>> could add our own s390 code to QEMU to issue VFIO_DEVICE_SET_IRQS
>>>> directly for an interpreted device, but I think would also need
>>>> s390-specific changes to VFIO_DEVICE_SET_IRQS accommodate this (e.g.
>>>> maybe something like a VFIO_IRQ_SET_DATA_S390AEN where we can then
>>>> specify the aen information in vfio_irq_set.data -- or something else I
>>>
>>> Hi,
>>>
>>> yes this in VFIO_DEVICE_SET_IRQS is what I think should be done.
>>>
>>>> haven't though of yet) -- I can try to look at this some more and 
>>>> see if
>>>> I get a good idea.
>>>
>>>
>>> I understood that the demand was concerning the IOMMU but I may be 
>>> wrong.

The IOMMU was an issue, but the request to move the ioctl out of vfio to 
kvm was specifically because these ioctl operations were only relevant 
for VMs and are not applicable to vfio uses cases outside of virtualization.

https://lore.kernel.org/kvm/20220208185141.GH4160@nvidia.com/

>>> For my opinion, the handling of AEN is not specific to KVM but specific
>>> to the device, for example the code should be the same if Z ever decide
>>> to use XEN or another hypervizor, except for the GISA part but this part
>>> is already implemented in KVM in a way it can be used from a device like
>>> in VFIO AP.


Fundamentally, these operations are valid only when you have _both_ a 
virtual machine and vfio device.  (Yes, you could swap in a new 
hypervisor with a new GISA implementation, but at the end of it the 
hypervisor must still provide the GISA designation for this to work)

If fh lookup is a concern, one idea that Jason floated was passing the 
vfio device fd as an argument to the kvm ioctl (so pass this down on a 
kvm ioctl from userspace instead of a fh) and then using a new vfio 
external API to get the relevant device from the provided fd.

https://lore.kernel.org/kvm/20220208195117.GI4160@nvidia.com/

>>>
>>> @Alex, what do you think?
>>>
>>> Regards,
>>> Pierre
>>>
>>
>> As I understand it the question isn't if it is specific to KVM but
>> rather if it is specific to virtualization. As vfio-pci is also used
>> for non virtualization purposes such as with DPDK/SPDK or a fully
>> emulating QEMU, it should only be in VFIO if it is relevant for these
>> kinds of user-space PCI accesses too. I'm not an AEN expert but as I
>> understand it, this does forwarding interrupts into a SIE context which
>> only makes sense for virtualization not for general user-space PCI.

Right, AEN forwarding is only relevant for virtual machines.

>>
> 
> Being in VFIO kernel part does not mean that this part should be called 
> from any user of VFIO in userland.
> That is a reason why I did propose an extension and not using the 
> current implementation of VFIO_DEVICE_SET_IRQS as is.
> 
> The reason behind is that the AEN hardware handling is device specific: 
> we need the Function Handle to program AEN.

You also need the GISA designation which is provided by the kvm or you 
also can't program AEN.  So you ultimately need both a function handle 
that is 'owned' by the device (vfio device fd) and the GISA designation 
that is 'owned' by kvm (kvm fd).  So there are 2 different "owning" fds 
involved.

> 
> If the API is through KVM which is device agnostic the implementation in 
> KVM has to search through the system to find the device being handled to 
> apply AEN on it.

See comment above about instead passing the vfio device fd.

> 
> This not the logical way for me and it is a potential source of problems 
> for future extensions.
>
Pierre Morel May 3, 2022, 2:53 p.m. UTC | #7
On 5/2/22 21:57, Matthew Rosato wrote:
> On 5/2/22 7:30 AM, Pierre Morel wrote:
>>
>>
>> On 5/2/22 11:19, Niklas Schnelle wrote:
>>> On Mon, 2022-05-02 at 09:48 +0200, Pierre Morel wrote:
>>>>
>>>> On 4/22/22 14:10, Matthew Rosato wrote:
>>>>> On 4/22/22 5:39 AM, Pierre Morel wrote:
>>>>>>
>>>>>> On 4/4/22 20:17, Matthew Rosato wrote:
>>>>>>> Use the associated kvm ioctl operation to enable adapter event
>>>>>>> notification
>>>>>>> and forwarding for devices when requested.  This feature will be 
>>>>>>> set up
>>>>>>> with or without firmware assist based upon the 'forwarding_assist'
>>>>>>> setting.
>>>>>>>
>>>>>>> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
>>>>>>> ---
>>>>>>>    hw/s390x/s390-pci-bus.c         | 20 ++++++++++++++---
>>>>>>>    hw/s390x/s390-pci-inst.c        | 40 
>>>>>>> +++++++++++++++++++++++++++++++--
>>>>>>>    hw/s390x/s390-pci-kvm.c         | 30 +++++++++++++++++++++++++
>>>>>>>    include/hw/s390x/s390-pci-bus.h |  1 +
>>>>>>>    include/hw/s390x/s390-pci-kvm.h | 14 ++++++++++++
>>>>>>>    5 files changed, 100 insertions(+), 5 deletions(-)
>>>>>>>
>>>>>>> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
>>>>>>> index 9c02d31250..47918d2ce9 100644
>>>>>>> --- a/hw/s390x/s390-pci-bus.c
>>>>>>> +++ b/hw/s390x/s390-pci-bus.c
>>>>>>> @@ -190,7 +190,10 @@ void s390_pci_sclp_deconfigure(SCCB *sccb)
>>>>>>>            rc = SCLP_RC_NO_ACTION_REQUIRED;
>>>>>>>            break;
>>>>>>>        default:
>>>>>>> -        if (pbdev->summary_ind) {
>>>>>>> +        if (pbdev->interp && (pbdev->fh & FH_MASK_ENABLE)) {
>>>>>>> +            /* Interpreted devices were using interrupt 
>>>>>>> forwarding */
>>>>>>> +            s390_pci_kvm_aif_disable(pbdev);
>>>>>>
>>>>>> Same remark as for the kernel part.
>>>>>> The VFIO device is already initialized and the action is on this
>>>>>> device, Shouldn't we use the VFIO device interface instead of the KVM
>>>>>> interface?
>>>>>>
>>>>>
>>>>> I don't necessarily disagree, but in v3 of the kernel series I was 
>>>>> told
>>>>> not to use VFIO ioctls to accomplish tasks that are unique to KVM 
>>>>> (e.g.
>>>>> AEN interpretation) and to instead use a KVM ioctl.
>>>>>
>>>>> VFIO_DEVICE_SET_IRQS won't work as-is for reasons described in the
>>>>> kernel series (e.g. we don't see any of the config space notifiers
>>>>> because of instruction interpretation) -- as far as I can figure we
>>>>> could add our own s390 code to QEMU to issue VFIO_DEVICE_SET_IRQS
>>>>> directly for an interpreted device, but I think would also need
>>>>> s390-specific changes to VFIO_DEVICE_SET_IRQS accommodate this (e.g.
>>>>> maybe something like a VFIO_IRQ_SET_DATA_S390AEN where we can then
>>>>> specify the aen information in vfio_irq_set.data -- or something 
>>>>> else I
>>>>
>>>> Hi,
>>>>
>>>> yes this in VFIO_DEVICE_SET_IRQS is what I think should be done.
>>>>
>>>>> haven't though of yet) -- I can try to look at this some more and 
>>>>> see if
>>>>> I get a good idea.
>>>>
>>>>
>>>> I understood that the demand was concerning the IOMMU but I may be 
>>>> wrong.
> 
> The IOMMU was an issue, but the request to move the ioctl out of vfio to 
> kvm was specifically because these ioctl operations were only relevant 
> for VMs and are not applicable to vfio uses cases outside of 
> virtualization.
> 
> https://lore.kernel.org/kvm/20220208185141.GH4160@nvidia.com/

I absolutely agree that KVM specific handling should go through KVM fd.
But as I say here under, AEN is not KVM specific but device specific.
Instruction interpretation is KVM specific.
see later---v

> 
>>>> For my opinion, the handling of AEN is not specific to KVM but specific
>>>> to the device, for example the code should be the same if Z ever decide
>>>> to use XEN or another hypervizor, except for the GISA part but this 
>>>> part
>>>> is already implemented in KVM in a way it can be used from a device 
>>>> like
>>>> in VFIO AP.
> 
> 
> Fundamentally, these operations are valid only when you have _both_ a 
> virtual machine and vfio device.  (Yes, you could swap in a new 
> hypervisor with a new GISA implementation, but at the end of it the 
> hypervisor must still provide the GISA designation for this to work)
> 
> If fh lookup is a concern, one idea that Jason floated was passing the 
> vfio device fd as an argument to the kvm ioctl (so pass this down on a 
> kvm ioctl from userspace instead of a fh) and then using a new vfio 
> external API to get the relevant device from the provided fd.
> 
> https://lore.kernel.org/kvm/20220208195117.GI4160@nvidia.com/

^------
This looks like a wrong architecture to me.

If something is used to virtualize the I/O of a device it should go 
through the device VFIO fd.

If we need a new VFIO external API why not using an extension of the 
VFIO_DEVICE_SET_IRQS and use directly the VFIO device to setup interrupts?

see following ----v

> 
>>>>
>>>> @Alex, what do you think?
>>>>
>>>> Regards,
>>>> Pierre
>>>>
>>>
>>> As I understand it the question isn't if it is specific to KVM but
>>> rather if it is specific to virtualization. As vfio-pci is also used
>>> for non virtualization purposes such as with DPDK/SPDK or a fully
>>> emulating QEMU, it should only be in VFIO if it is relevant for these
>>> kinds of user-space PCI accesses too. I'm not an AEN expert but as I
>>> understand it, this does forwarding interrupts into a SIE context which
>>> only makes sense for virtualization not for general user-space PCI.
> 
> Right, AEN forwarding is only relevant for virtual machines.
> 
>>>
>>
>> Being in VFIO kernel part does not mean that this part should be 
>> called from any user of VFIO in userland.
>> That is a reason why I did propose an extension and not using the 
>> current implementation of VFIO_DEVICE_SET_IRQS as is.
>>
>> The reason behind is that the AEN hardware handling is device 
>> specific: we need the Function Handle to program AEN.
> 
> You also need the GISA designation which is provided by the kvm or you 
> also can't program AEN.  So you ultimately need both a function handle 
> that is 'owned' by the device (vfio device fd) and the GISA designation 
> that is 'owned' by kvm (kvm fd).  So there are 2 different "owning" fds 
> involved.

Yes GISA is a host structure, not device specific but guest specific and 
exist very soon during the guest creation, there should be no problem to 
retrieve it from a VFIO device IOTCL.

> 
>>
>> If the API is through KVM which is device agnostic the implementation 
>> in KVM has to search through the system to find the device being 
>> handled to apply AEN on it.
> 
> See comment above about instead passing the vfio device fd.
> 
>>
>> This not the logical way for me and it is a potential source of 
>> problems for future extensions.
>>
> 
> 
>
^------

There are three different things to modify for the Z-guest to use VFIO:
- IOMMU
- device IRQ
- instruction interpretation, feature negociation

For my opinion only the last one should go directly through the KVM fd.

This should be possible for all architectures.
If it is not possible for Z, the failing path must be adapted it should 
not go through another path.

Giving the right IRQ information to the host can be done with a 
dedicated IOCTL through the VFIO device fd, just like we need an 
extension in the other direction to retrieve the Z specific capabilities.

I am quite sure that other architectures will need some specificity too 
for the interrupt or IOMMU handling in the future with increasing 
implementation of virtualization in the firmware.

Having a dedicated IOCTL command means it can be called from QEMU and 
for guest virtualizuation only then let unused for other userland access.


Regards,
Pierre
Matthew Rosato May 4, 2022, 2:20 p.m. UTC | #8
On 5/3/22 10:53 AM, Pierre Morel wrote:
> 
> 
> On 5/2/22 21:57, Matthew Rosato wrote:
>> On 5/2/22 7:30 AM, Pierre Morel wrote:
>>>
>>>
>>> On 5/2/22 11:19, Niklas Schnelle wrote:
>>>> On Mon, 2022-05-02 at 09:48 +0200, Pierre Morel wrote:
>>>>>
>>>>> On 4/22/22 14:10, Matthew Rosato wrote:
>>>>>> On 4/22/22 5:39 AM, Pierre Morel wrote:
>>>>>>>
>>>>>>> On 4/4/22 20:17, Matthew Rosato wrote:
>>>>>>>> Use the associated kvm ioctl operation to enable adapter event
>>>>>>>> notification
>>>>>>>> and forwarding for devices when requested.  This feature will be 
>>>>>>>> set up
>>>>>>>> with or without firmware assist based upon the 'forwarding_assist'
>>>>>>>> setting.
>>>>>>>>
>>>>>>>> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
>>>>>>>> ---
>>>>>>>>    hw/s390x/s390-pci-bus.c         | 20 ++++++++++++++---
>>>>>>>>    hw/s390x/s390-pci-inst.c        | 40 
>>>>>>>> +++++++++++++++++++++++++++++++--
>>>>>>>>    hw/s390x/s390-pci-kvm.c         | 30 +++++++++++++++++++++++++
>>>>>>>>    include/hw/s390x/s390-pci-bus.h |  1 +
>>>>>>>>    include/hw/s390x/s390-pci-kvm.h | 14 ++++++++++++
>>>>>>>>    5 files changed, 100 insertions(+), 5 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
>>>>>>>> index 9c02d31250..47918d2ce9 100644
>>>>>>>> --- a/hw/s390x/s390-pci-bus.c
>>>>>>>> +++ b/hw/s390x/s390-pci-bus.c
>>>>>>>> @@ -190,7 +190,10 @@ void s390_pci_sclp_deconfigure(SCCB *sccb)
>>>>>>>>            rc = SCLP_RC_NO_ACTION_REQUIRED;
>>>>>>>>            break;
>>>>>>>>        default:
>>>>>>>> -        if (pbdev->summary_ind) {
>>>>>>>> +        if (pbdev->interp && (pbdev->fh & FH_MASK_ENABLE)) {
>>>>>>>> +            /* Interpreted devices were using interrupt 
>>>>>>>> forwarding */
>>>>>>>> +            s390_pci_kvm_aif_disable(pbdev);
>>>>>>>
>>>>>>> Same remark as for the kernel part.
>>>>>>> The VFIO device is already initialized and the action is on this
>>>>>>> device, Shouldn't we use the VFIO device interface instead of the 
>>>>>>> KVM
>>>>>>> interface?
>>>>>>>
>>>>>>
>>>>>> I don't necessarily disagree, but in v3 of the kernel series I was 
>>>>>> told
>>>>>> not to use VFIO ioctls to accomplish tasks that are unique to KVM 
>>>>>> (e.g.
>>>>>> AEN interpretation) and to instead use a KVM ioctl.
>>>>>>
>>>>>> VFIO_DEVICE_SET_IRQS won't work as-is for reasons described in the
>>>>>> kernel series (e.g. we don't see any of the config space notifiers
>>>>>> because of instruction interpretation) -- as far as I can figure we
>>>>>> could add our own s390 code to QEMU to issue VFIO_DEVICE_SET_IRQS
>>>>>> directly for an interpreted device, but I think would also need
>>>>>> s390-specific changes to VFIO_DEVICE_SET_IRQS accommodate this (e.g.
>>>>>> maybe something like a VFIO_IRQ_SET_DATA_S390AEN where we can then
>>>>>> specify the aen information in vfio_irq_set.data -- or something 
>>>>>> else I
>>>>>
>>>>> Hi,
>>>>>
>>>>> yes this in VFIO_DEVICE_SET_IRQS is what I think should be done.
>>>>>
>>>>>> haven't though of yet) -- I can try to look at this some more and 
>>>>>> see if
>>>>>> I get a good idea.
>>>>>
>>>>>
>>>>> I understood that the demand was concerning the IOMMU but I may be 
>>>>> wrong.
>>
>> The IOMMU was an issue, but the request to move the ioctl out of vfio 
>> to kvm was specifically because these ioctl operations were only 
>> relevant for VMs and are not applicable to vfio uses cases outside of 
>> virtualization.
>>
>> https://lore.kernel.org/kvm/20220208185141.GH4160@nvidia.com/
> 
> I absolutely agree that KVM specific handling should go through KVM fd.
> But as I say here under, AEN is not KVM specific but device specific.
> Instruction interpretation is KVM specific.
> see later---v
> 
>>
>>>>> For my opinion, the handling of AEN is not specific to KVM but 
>>>>> specific
>>>>> to the device, for example the code should be the same if Z ever 
>>>>> decide
>>>>> to use XEN or another hypervizor, except for the GISA part but this 
>>>>> part
>>>>> is already implemented in KVM in a way it can be used from a device 
>>>>> like
>>>>> in VFIO AP.
>>
>>
>> Fundamentally, these operations are valid only when you have _both_ a 
>> virtual machine and vfio device.  (Yes, you could swap in a new 
>> hypervisor with a new GISA implementation, but at the end of it the 
>> hypervisor must still provide the GISA designation for this to work)
>>
>> If fh lookup is a concern, one idea that Jason floated was passing the 
>> vfio device fd as an argument to the kvm ioctl (so pass this down on a 
>> kvm ioctl from userspace instead of a fh) and then using a new vfio 
>> external API to get the relevant device from the provided fd.
>>
>> https://lore.kernel.org/kvm/20220208195117.GI4160@nvidia.com/
> 
> ^------
> This looks like a wrong architecture to me.
> 
> If something is used to virtualize the I/O of a device it should go 
> through the device VFIO fd.
> 
> If we need a new VFIO external API why not using an extension of the 
> VFIO_DEVICE_SET_IRQS and use directly the VFIO device to setup interrupts?
> 
> see following ----v
> 
>>
>>>>>
>>>>> @Alex, what do you think?
>>>>>
>>>>> Regards,
>>>>> Pierre
>>>>>
>>>>
>>>> As I understand it the question isn't if it is specific to KVM but
>>>> rather if it is specific to virtualization. As vfio-pci is also used
>>>> for non virtualization purposes such as with DPDK/SPDK or a fully
>>>> emulating QEMU, it should only be in VFIO if it is relevant for these
>>>> kinds of user-space PCI accesses too. I'm not an AEN expert but as I
>>>> understand it, this does forwarding interrupts into a SIE context which
>>>> only makes sense for virtualization not for general user-space PCI.
>>
>> Right, AEN forwarding is only relevant for virtual machines.
>>
>>>>
>>>
>>> Being in VFIO kernel part does not mean that this part should be 
>>> called from any user of VFIO in userland.
>>> That is a reason why I did propose an extension and not using the 
>>> current implementation of VFIO_DEVICE_SET_IRQS as is.
>>>
>>> The reason behind is that the AEN hardware handling is device 
>>> specific: we need the Function Handle to program AEN.
>>
>> You also need the GISA designation which is provided by the kvm or you 
>> also can't program AEN.  So you ultimately need both a function handle 
>> that is 'owned' by the device (vfio device fd) and the GISA 
>> designation that is 'owned' by kvm (kvm fd).  So there are 2 different 
>> "owning" fds involved.
> 
> Yes GISA is a host structure, not device specific but guest specific and 
> exist very soon during the guest creation, there should be no problem to 
> retrieve it from a VFIO device IOTCL.
> 
>>
>>>
>>> If the API is through KVM which is device agnostic the implementation 
>>> in KVM has to search through the system to find the device being 
>>> handled to apply AEN on it.
>>
>> See comment above about instead passing the vfio device fd.
>>
>>>
>>> This not the logical way for me and it is a potential source of 
>>> problems for future extensions.
>>>
>>
>>
>>
> ^------
> 
> There are three different things to modify for the Z-guest to use VFIO:
> - IOMMU
> - device IRQ
> - instruction interpretation, feature negociation
> 
> For my opinion only the last one should go directly through the KVM fd.
> 
> This should be possible for all architectures.
> If it is not possible for Z, the failing path must be adapted it should 
> not go through another path.
> 
> Giving the right IRQ information to the host can be done with a 
> dedicated IOCTL through the VFIO device fd, just like we need an 
> extension in the other direction to retrieve the Z specific capabilities.
> 
> I am quite sure that other architectures will need some specificity too 
> for the interrupt or IOMMU handling in the future with increasing 
> implementation of virtualization in the firmware.
> 
> Having a dedicated IOCTL command means it can be called from QEMU and 
> for guest virtualizuation only then let unused for other userland access.
> 

Another approach (that I admittedly don't have all the details worked 
out on yet) would be to do something like add a new type of 
kvm_irq_routing_entry that can be used specifically for AEN.  Then we 
can establish this route with the following info:

struct kvm_irq_routing_s390_aen {
        __u64 ind_addr;
        __u64 summary_addr;
        __u32 fd; /* vfio device fd */
        __u32 noi;
        __u8  isc;
        __u8  sbo;
};

The vfio device fd is required as it would then be used to get the 
associated zdev and thus its fh, which we need when we go to activate 
AEN (mpcifc).  Our existing adapter-based routes 
(kvm_irq_routing_s390_adapter) lack this association and I can't think 
of a way around that besides introducing a different route type.

During interrupt.c:kvm_set_routing_entry() we can stash the guest info + 
fd.  Then during irq_bypass_{add,del}_producer for this new route type 
we can find the zpci->fh via the vfio device fd and then actually 
(un)pin guest addresses / issue the host mpcfic using the routing info. 
vfio will trigger irq_bypass_{add,del}_producer when the virq is 
enabled/disabled.

 From QEMU though, it gets a bit weird -- since we enable load/store 
interpretation, we will never see any of the config space writes from 
the guest.  So, vfio MSI notifiers never get triggered to call e.g. 
kvm_irqchip_add_msi_route -- we would have to do this ourselves in 
s390x-pci, specifying the new route type above.  And then also possibly 
issue our own VFIO_DEVICE_SET_IRQS since again, it will never get 
tripped via a vfio notifier.

Or alternatively, we can intentionally trigger the MSI notifiers from 
s390x-pci code (looks like spapr does this via spapr_msi_setmsg) for the 
number of vectors the guest specifies on the mpcific to create the 
necessary virq(s) and drive the subsequent VFIO_DEVICE_SET_IRQ call. 
Actually, we might have to do that anyway to satisfy 
VFIO_DEVICE_SET_IRQS expectations in the kernel.  And, using the above 
structure, we probably only need to create a single virq since it 
contains all of the routes in one payload + once AEN is established we 
are always delivering interrupts to the guest via GISA, not over eventfd.
diff mbox series

Patch

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 9c02d31250..47918d2ce9 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -190,7 +190,10 @@  void s390_pci_sclp_deconfigure(SCCB *sccb)
         rc = SCLP_RC_NO_ACTION_REQUIRED;
         break;
     default:
-        if (pbdev->summary_ind) {
+        if (pbdev->interp && (pbdev->fh & FH_MASK_ENABLE)) {
+            /* Interpreted devices were using interrupt forwarding */
+            s390_pci_kvm_aif_disable(pbdev);
+        } else if (pbdev->summary_ind) {
             pci_dereg_irqs(pbdev);
         }
         if (pbdev->iommu->enabled) {
@@ -1078,6 +1081,7 @@  static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
                 } else {
                     DPRINTF("zPCI interpretation facilities missing.\n");
                     pbdev->interp = false;
+                    pbdev->forwarding_assist = false;
                 }
             }
             pbdev->iommu->dma_limit = s390_pci_start_dma_count(s, pbdev);
@@ -1086,11 +1090,13 @@  static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
             if (!pbdev->interp) {
                 /* Do vfio passthrough but intercept for I/O */
                 pbdev->fh |= FH_SHM_VFIO;
+                pbdev->forwarding_assist = false;
             }
         } else {
             pbdev->fh |= FH_SHM_EMUL;
             /* Always intercept emulated devices */
             pbdev->interp = false;
+            pbdev->forwarding_assist = false;
         }
 
         if (s390_pci_msix_init(pbdev) && !pbdev->interp) {
@@ -1240,7 +1246,10 @@  static void s390_pcihost_reset(DeviceState *dev)
     /* Process all pending unplug requests */
     QTAILQ_FOREACH_SAFE(pbdev, &s->zpci_devs, link, next) {
         if (pbdev->unplug_requested) {
-            if (pbdev->summary_ind) {
+            if (pbdev->interp && (pbdev->fh & FH_MASK_ENABLE)) {
+                /* Interpreted devices were using interrupt forwarding */
+                s390_pci_kvm_aif_disable(pbdev);
+            } else if (pbdev->summary_ind) {
                 pci_dereg_irqs(pbdev);
             }
             if (pbdev->iommu->enabled) {
@@ -1378,7 +1387,10 @@  static void s390_pci_device_reset(DeviceState *dev)
         break;
     }
 
-    if (pbdev->summary_ind) {
+    if (pbdev->interp && (pbdev->fh & FH_MASK_ENABLE)) {
+        /* Interpreted devices were using interrupt forwarding */
+        s390_pci_kvm_aif_disable(pbdev);
+    } else if (pbdev->summary_ind) {
         pci_dereg_irqs(pbdev);
     }
     if (pbdev->iommu->enabled) {
@@ -1424,6 +1436,8 @@  static Property s390_pci_device_properties[] = {
     DEFINE_PROP_S390_PCI_FID("fid", S390PCIBusDevice, fid),
     DEFINE_PROP_STRING("target", S390PCIBusDevice, target),
     DEFINE_PROP_BOOL("interpret", S390PCIBusDevice, interp, true),
+    DEFINE_PROP_BOOL("forwarding_assist", S390PCIBusDevice, forwarding_assist,
+                     true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index c898c8abe9..c3a34da73d 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -1062,6 +1062,32 @@  static void fmb_update(void *opaque)
     timer_mod(pbdev->fmb_timer, t + pbdev->pci_group->zpci_group.mui);
 }
 
+static int mpcifc_reg_int_interp(S390PCIBusDevice *pbdev, ZpciFib *fib)
+{
+    int rc;
+
+    rc = s390_pci_kvm_aif_enable(pbdev, fib, pbdev->forwarding_assist);
+    if (rc) {
+        DPRINTF("Failed to enable interrupt forwarding\n");
+        return rc;
+    }
+
+    return 0;
+}
+
+static int mpcifc_dereg_int_interp(S390PCIBusDevice *pbdev, ZpciFib *fib)
+{
+    int rc;
+
+    rc = s390_pci_kvm_aif_disable(pbdev);
+    if (rc) {
+        DPRINTF("Failed to disable interrupt forwarding\n");
+        return rc;
+    }
+
+    return 0;
+}
+
 int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar,
                         uintptr_t ra)
 {
@@ -1116,7 +1142,12 @@  int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar,
 
     switch (oc) {
     case ZPCI_MOD_FC_REG_INT:
-        if (pbdev->summary_ind) {
+        if (pbdev->interp) {
+            if (mpcifc_reg_int_interp(pbdev, &fib)) {
+                cc = ZPCI_PCI_LS_ERR;
+                s390_set_status_code(env, r1, ZPCI_MOD_ST_SEQUENCE);
+            }
+        } else if (pbdev->summary_ind) {
             cc = ZPCI_PCI_LS_ERR;
             s390_set_status_code(env, r1, ZPCI_MOD_ST_SEQUENCE);
         } else if (reg_irqs(env, pbdev, fib)) {
@@ -1125,7 +1156,12 @@  int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar,
         }
         break;
     case ZPCI_MOD_FC_DEREG_INT:
-        if (!pbdev->summary_ind) {
+        if (pbdev->interp) {
+            if (mpcifc_dereg_int_interp(pbdev, &fib)) {
+                cc = ZPCI_PCI_LS_ERR;
+                s390_set_status_code(env, r1, ZPCI_MOD_ST_SEQUENCE);
+            }
+        } else if (!pbdev->summary_ind) {
             cc = ZPCI_PCI_LS_ERR;
             s390_set_status_code(env, r1, ZPCI_MOD_ST_SEQUENCE);
         } else {
diff --git a/hw/s390x/s390-pci-kvm.c b/hw/s390x/s390-pci-kvm.c
index 8bfce9ef18..cb20b8dcb9 100644
--- a/hw/s390x/s390-pci-kvm.c
+++ b/hw/s390x/s390-pci-kvm.c
@@ -11,11 +11,41 @@ 
 
 #include "qemu/osdep.h"
 
+#include <linux/kvm.h>
+
 #include "kvm/kvm_s390x.h"
+#include "hw/s390x/s390-pci-bus.h"
 #include "hw/s390x/s390-pci-kvm.h"
+#include "hw/s390x/s390-pci-inst.h"
 #include "cpu_models.h"
 
 bool s390_pci_kvm_interp_allowed(void)
 {
     return s390_has_feat(S390_FEAT_ZPCI_INTERP) && kvm_s390_get_zpci_op();
 }
+
+int s390_pci_kvm_aif_enable(S390PCIBusDevice *pbdev, ZpciFib *fib, bool assist)
+{
+    struct kvm_s390_zpci_op args = {
+        .fh = pbdev->fh,
+        .op = KVM_S390_ZPCIOP_REG_AEN,
+        .u.reg_aen.ibv = fib->aibv,
+        .u.reg_aen.sb = fib->aisb,
+        .u.reg_aen.noi = FIB_DATA_NOI(fib->data),
+        .u.reg_aen.isc = FIB_DATA_ISC(fib->data),
+        .u.reg_aen.sbo = FIB_DATA_AISBO(fib->data),
+        .u.reg_aen.flags = (assist) ? 0 : KVM_S390_ZPCIOP_REGAEN_HOST
+    };
+
+    return kvm_vm_ioctl(kvm_state, KVM_S390_ZPCI_OP, &args);
+}
+
+int s390_pci_kvm_aif_disable(S390PCIBusDevice *pbdev)
+{
+    struct kvm_s390_zpci_op args = {
+        .fh = pbdev->fh,
+        .op = KVM_S390_ZPCIOP_DEREG_AEN
+    };
+
+    return kvm_vm_ioctl(kvm_state, KVM_S390_ZPCI_OP, &args);
+}
diff --git a/include/hw/s390x/s390-pci-bus.h b/include/hw/s390x/s390-pci-bus.h
index a9843dfe97..5b09f0cf2f 100644
--- a/include/hw/s390x/s390-pci-bus.h
+++ b/include/hw/s390x/s390-pci-bus.h
@@ -351,6 +351,7 @@  struct S390PCIBusDevice {
     bool pci_unplug_request_processed;
     bool unplug_requested;
     bool interp;
+    bool forwarding_assist;
     QTAILQ_ENTRY(S390PCIBusDevice) link;
 };
 
diff --git a/include/hw/s390x/s390-pci-kvm.h b/include/hw/s390x/s390-pci-kvm.h
index 80a2e7d0ca..933814a402 100644
--- a/include/hw/s390x/s390-pci-kvm.h
+++ b/include/hw/s390x/s390-pci-kvm.h
@@ -12,13 +12,27 @@ 
 #ifndef HW_S390_PCI_KVM_H
 #define HW_S390_PCI_KVM_H
 
+#include "hw/s390x/s390-pci-bus.h"
+#include "hw/s390x/s390-pci-inst.h"
+
 #ifdef CONFIG_KVM
 bool s390_pci_kvm_interp_allowed(void);
+int s390_pci_kvm_aif_enable(S390PCIBusDevice *pbdev, ZpciFib *fib, bool assist);
+int s390_pci_kvm_aif_disable(S390PCIBusDevice *pbdev);
 #else
 static inline bool s390_pci_kvm_interp_allowed(void)
 {
     return false;
 }
+static inline int s390_pci_kvm_aif_enable(S390PCIBusDevice *pbdev, ZpciFib *fib,
+                                          bool assist)
+{
+    return -EINVAL;
+}
+static inline int s390_pci_kvm_aif_disable(S390PCIBusDevice *pbdev)
+{
+    return -EINVAL;
+}
 #endif
 
 #endif