On 11/09/2023 09:57, Duan, Zhenzhong wrote:
>> -----Original Message-----
>> From: qemu-devel-bounces+zhenzhong.duan=intel....@nongnu.org <qemu-
>> devel-bounces+zhenzhong.duan=intel....@nongnu.org> On Behalf Of Joao
>> Martins
>> Sent: Friday, September 8, 2023 5:30 PM
>> Subject: [PATCH v1] vfio/common: Separate vfio-pci ranges
>>
>> QEMU computes the DMA logging ranges for two predefined ranges: 32-bit
>> and 64-bit. In the OVMF case, when the dynamic MMIO window is enabled,
>> QEMU includes in the 64-bit range the RAM regions at the lower part
>> and vfio-pci device RAM regions which are at the top of the address
>> space. This range contains a large gap and the size can be bigger than
>> the dirty tracking HW limits of some devices (MLX5 has a 2^42 limit).
>>
>> To avoid such large ranges, introduce a new PCI range covering the
>> vfio-pci device RAM regions, this only if the addresses are above 4GB
>> to avoid breaking potential SeaBIOS guests.
>>
>> Signed-off-by: Joao Martins <joao.m.mart...@oracle.com>
>> [ clg: - wrote commit log
>> - fixed overlapping 32-bit and PCI ranges when using SeaBIOS ]
>> Signed-off-by: Cédric Le Goater <c...@redhat.com>
>> ---
>> v2:
>> * s/minpci/minpci64/
>> * s/maxpci/maxpci64/
>> * Expand comment to cover the pci-hole64 and why we don't do special
>> handling of pci-hole32.
>> ---
>> hw/vfio/common.c | 71 +++++++++++++++++++++++++++++++++++++-------
>> hw/vfio/trace-events | 2 +-
>> 2 files changed, 61 insertions(+), 12 deletions(-)
>>
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index 237101d03844..134649226d43 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -27,6 +27,7 @@
>>
>> #include "hw/vfio/vfio-common.h"
>> #include "hw/vfio/vfio.h"
>> +#include "hw/vfio/pci.h"
>> #include "exec/address-spaces.h"
>> #include "exec/memory.h"
>> #include "exec/ram_addr.h"
>> @@ -1400,6 +1401,8 @@ typedef struct VFIODirtyRanges {
>> hwaddr max32;
>> hwaddr min64;
>> hwaddr max64;
>> + hwaddr minpci64;
>> + hwaddr maxpci64;
>> } VFIODirtyRanges;
>>
>> typedef struct VFIODirtyRangesListener {
>> @@ -1408,6 +1411,31 @@ typedef struct VFIODirtyRangesListener {
>> MemoryListener listener;
>> } VFIODirtyRangesListener;
>>
>> +static bool vfio_section_is_vfio_pci(MemoryRegionSection *section,
>> + VFIOContainer *container)
>> +{
>> + VFIOPCIDevice *pcidev;
>> + VFIODevice *vbasedev;
>> + VFIOGroup *group;
>> + Object *owner;
>> +
>> + owner = memory_region_owner(section->mr);
>> +
>> + QLIST_FOREACH(group, &container->group_list, container_next) {
>> + QLIST_FOREACH(vbasedev, &group->device_list, next) {
>> + if (vbasedev->type != VFIO_DEVICE_TYPE_PCI) {
>> + continue;
>> + }
>> + pcidev = container_of(vbasedev, VFIOPCIDevice, vbasedev);
>> + if (OBJECT(pcidev) == owner) {
>> + return true;
>> + }
>> + }
>> + }
>> +
>> + return false;
>> +}
>
> What about simplify it with memory_region_is_ram_device()?
> This way vdpa device could also be included.
>
Note that the check is not interested in RAM (or any other kinda of memory like
VGA). That's covered in the 32-64 ranges. But rather in any PCI device RAM that
would fall in the 64-bit PCI hole. Would memory_region_is_ram_device() really
cover it? If so, I am all for the simplification.
Joao
