Re: [PATCH v5 7/9] s390x/pci: enable adapter event notification for interpreted devices
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
---
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.gh4...@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.gi4...@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
Re: [PATCH v5 7/9] s390x/pci: enable adapter event notification for interpreted devices
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
---
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.gh4...@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.gi4...@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
Re: [PATCH v5 7/9] s390x/pci: enable adapter event notification for interpreted devices
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
---
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.gh4...@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.gi4...@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
Re: [PATCH v5 7/9] s390x/pci: enable adapter event notification for interpreted devices
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
---
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
--
Pierre Morel
IBM Lab Boeblingen
Re: [PATCH v5 7/9] s390x/pci: enable adapter event notification for interpreted devices
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
> > > > ---
> > > > 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.
Re: [PATCH v5 7/9] s390x/pci: enable adapter event notification for interpreted devices
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
---
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
--
Pierre Morel
IBM Lab Boeblingen
Re: [PATCH v5 7/9] s390x/pci: enable adapter event notification for interpreted devices
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
---
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.
Re: [PATCH v5 7/9] s390x/pci: enable adapter event notification for interpreted devices
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
---
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
--
Pierre Morel
IBM Lab Boeblingen
[PATCH v5 7/9] s390x/pci: enable adapter event notification for interpreted devices
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
---
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);
+} 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, >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, )) {
+cc = ZPCI_PCI_LS_ERR;
+s390_set_status_code(env, r1, ZPCI_MOD_ST_SEQUENCE);
+}
+
