[PATCH 0/2] s390x/pci: small set of fixes

[PATCH 0/2] s390x/pci: small set of fixes

[Matthew Rosato]

The following set of changes are associated with issues exposed by testing
of the 'vfio: Adopt iommufd' series.

The first patch fixes an existing assumption that a vfio device will always
have a group fd (which is no longer true if cdev is used, which can only
happen once the iommufd backend is used).  This patch really only needs to
go into 8.2 if the 'vfio: Adopt iommufd' series does (but would be fine to 
go into 8.2 without it too).

The second patch fixes an issue where we do not detect that a vfio DMA limit
was never read from vfio.  This is actually an existing bug as it's possible
for an older host kernel to be missing this support today; so ideally this one
should be targeted for 8.2 regardless. 

Matthew Rosato (2):
  s390x/pci: bypass vfio DMA counting when using cdev
  s390x/pci: only limit DMA aperture if vfio DMA limit reported

 hw/s390x/s390-pci-vfio.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

-- 
2.41.0

[PATCH 1/2] s390x/pci: bypass vfio DMA counting when using cdev

[Matthew Rosato]

The current code assumes that there is always a vfio group, but
that's no longer guaranteed with the iommufd backend when using
cdev.  In this case, we don't need to track the vfio dma limit
anyway.

Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
---
 hw/s390x/s390-pci-vfio.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c
index 59a2e03873..7218583883 100644
--- a/hw/s390x/s390-pci-vfio.c
+++ b/hw/s390x/s390-pci-vfio.c
@@ -66,7 +66,11 @@ S390PCIDMACount *s390_pci_start_dma_count(S390pciState *s,
 
     assert(vpdev);
 
-    id = vpdev->vbasedev.group->container->fd;
+    if (vpdev->vbasedev.group) {
+        id = vpdev->vbasedev.group->container->fd;
+    } else {
+        return NULL;
+    }
 
     if (!s390_pci_update_dma_avail(id, &avail)) {
         return NULL;
-- 
2.41.0

Re: [PATCH 1/2] s390x/pci: bypass vfio DMA counting when using cdev

[Philippe Mathieu-Daudé]

On 9/11/23 23:53, Matthew Rosato wrote:
> The current code assumes that there is always a vfio group, but
> that's no longer guaranteed with the iommufd backend when using
> cdev.  In this case, we don't need to track the vfio dma limit
> anyway.
> 
> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
> ---
>   hw/s390x/s390-pci-vfio.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c
> index 59a2e03873..7218583883 100644
> --- a/hw/s390x/s390-pci-vfio.c
> +++ b/hw/s390x/s390-pci-vfio.c
> @@ -66,7 +66,11 @@ S390PCIDMACount *s390_pci_start_dma_count(S390pciState *s,
>   
>       assert(vpdev);
>   

Matter of taste, simpler as:

        if (!vpdev->vbasedev.group) {
            return NULL;
        }

> -    id = vpdev->vbasedev.group->container->fd;

and this line isn't changed.

> +    if (vpdev->vbasedev.group) {
> +        id = vpdev->vbasedev.group->container->fd;
> +    } else {
> +        return NULL;
> +    }
>   
>       if (!s390_pci_update_dma_avail(id, &avail)) {
>           return NULL;

Re: [PATCH 1/2] s390x/pci: bypass vfio DMA counting when using cdev

[Philippe Mathieu-Daudé]

[Sent too fast by inadvertence...]

Hi Matthew,

On 10/11/23 06:21, Philippe Mathieu-Daudé wrote:
> On 9/11/23 23:53, Matthew Rosato wrote:
>> The current code assumes that there is always a vfio group, but
>> that's no longer guaranteed with the iommufd backend when using
>> cdev.  In this case, we don't need to track the vfio dma limit
>> anyway.
>>
>> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
>> ---
>>   hw/s390x/s390-pci-vfio.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c
>> index 59a2e03873..7218583883 100644
>> --- a/hw/s390x/s390-pci-vfio.c
>> +++ b/hw/s390x/s390-pci-vfio.c
>> @@ -66,7 +66,11 @@ S390PCIDMACount 
>> *s390_pci_start_dma_count(S390pciState *s,
>>       assert(vpdev);
> 
> Matter of taste, simpler as:
> 
>         if (!vpdev->vbasedev.group) {
>             return NULL;
>         }
> 
>> -    id = vpdev->vbasedev.group->container->fd;
> 
> and this line isn't changed.
> 
>> +    if (vpdev->vbasedev.group) {
>> +        id = vpdev->vbasedev.group->container->fd;
>> +    } else {
>> +        return NULL;
>> +    }
>>       if (!s390_pci_update_dma_avail(id, &avail)) {
>>           return NULL;
> 

Regards,

Phil :)

[PATCH 2/2] s390x/pci: only limit DMA aperture if vfio DMA limit reported

[Matthew Rosato]

If the host kernel lacks vfio DMA limit reporting, do not attempt
to shrink the guest DMA aperture.

Fixes: df202e3ff3 ("s390x/pci: shrink DMA aperture to be bound by vfio DMA limit")
Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
---
 hw/s390x/s390-pci-vfio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c
index 7218583883..22b39a7d18 100644
--- a/hw/s390x/s390-pci-vfio.c
+++ b/hw/s390x/s390-pci-vfio.c
@@ -136,7 +136,7 @@ static void s390_pci_read_base(S390PCIBusDevice *pbdev,
      * to the guest based upon the vfio DMA limit.
      */
     vfio_size = pbdev->iommu->max_dma_limit << TARGET_PAGE_BITS;
-    if (vfio_size < (cap->end_dma - cap->start_dma + 1)) {
+    if ((vfio_size > 0) && (vfio_size < (cap->end_dma - cap->start_dma + 1))) {
         pbdev->zpci_fn.edma = cap->start_dma + vfio_size - 1;
     }
 }
-- 
2.41.0

Re: [PATCH 2/2] s390x/pci: only limit DMA aperture if vfio DMA limit reported

[Thomas Huth]

On 09/11/2023 23.53, Matthew Rosato wrote:
> If the host kernel lacks vfio DMA limit reporting, do not attempt
> to shrink the guest DMA aperture.
> 
> Fixes: df202e3ff3 ("s390x/pci: shrink DMA aperture to be bound by vfio DMA limit")
> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
> ---
>   hw/s390x/s390-pci-vfio.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c
> index 7218583883..22b39a7d18 100644
> --- a/hw/s390x/s390-pci-vfio.c
> +++ b/hw/s390x/s390-pci-vfio.c
> @@ -136,7 +136,7 @@ static void s390_pci_read_base(S390PCIBusDevice *pbdev,
>        * to the guest based upon the vfio DMA limit.
>        */
>       vfio_size = pbdev->iommu->max_dma_limit << TARGET_PAGE_BITS;
> -    if (vfio_size < (cap->end_dma - cap->start_dma + 1)) {
> +    if ((vfio_size > 0) && (vfio_size < (cap->end_dma - cap->start_dma + 1))) {

Please drop the superfluous parantheses:

  if (vfio_size > 0 && vfio_size < cap->end_dma - cap->start_dma + 1) {

Thanks,
   Thomas


>           pbdev->zpci_fn.edma = cap->start_dma + vfio_size - 1;
>       }
>   }

Re: [PATCH 0/2] s390x/pci: small set of fixes

[Cédric Le Goater]

Hello Matthew,

On 11/9/23 23:53, Matthew Rosato wrote:
> The following set of changes are associated with issues exposed by testing
> of the 'vfio: Adopt iommufd' series.
> 
> The first patch fixes an existing assumption that a vfio device will always
> have a group fd (which is no longer true if cdev is used, which can only
> happen once the iommufd backend is used).  This patch really only needs to
> go into 8.2 if the 'vfio: Adopt iommufd' series does (but would be fine to
> go into 8.2 without it too).

It is a bit late for the 8.2. So we have time to polish v5 of the 'vfio: Adopt
iommufd' series a bit more. I should include the v6 in an early PR for a 9.0.
  
> The second patch fixes an issue where we do not detect that a vfio DMA limit
> was never read from vfio.  This is actually an existing bug as it's possible
> for an older host kernel to be missing this support today; so ideally this one
> should be targeted for 8.2 regardless.

Nevertheless, I hope these two fixes can reached 8.2 since they are good
to have anyhow.

Thanks,

C.

  
> Matthew Rosato (2):
>    s390x/pci: bypass vfio DMA counting when using cdev
>    s390x/pci: only limit DMA aperture if vfio DMA limit reported
> 
>   hw/s390x/s390-pci-vfio.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
>