Re: [PATCH V7 vfio 07/10] vfio/mlx5: Create and destroy page tracker object

[Joao Martins <joao.m.martins@oracle.com>]

On 07/09/2023 10:56, Cédric Le Goater wrote:
> On 9/6/23 13:51, Jason Gunthorpe wrote:
>> On Wed, Sep 06, 2023 at 10:55:26AM +0200, Cédric Le Goater wrote:
>>
>>>> +    WARN_ON(node);
>>>> +    log_addr_space_size = ilog2(total_ranges_len);
>>>> +    if (log_addr_space_size <
>>>> +        (MLX5_CAP_ADV_VIRTUALIZATION(mdev, pg_track_log_min_addr_space)) ||
>>>> +        log_addr_space_size >
>>>> +        (MLX5_CAP_ADV_VIRTUALIZATION(mdev, pg_track_log_max_addr_space))) {
>>>> +        err = -EOPNOTSUPP;
>>>> +        goto out;
>>>> +    }
>>>
>>>
>>> We are seeing an issue with dirty page tracking when doing migration
>>> of an OVMF VM guest. The vfio-pci variant driver for the MLX5 VF
>>> device complains when dirty page tracking is initialized from QEMU :
>>>
>>>    qemu-kvm: 0000:b1:00.2: Failed to start DMA logging, err -95 (Operation
>>> not supported)
>>>
>>> The 64-bit computed range is  :
>>>
>>>    vfio_device_dirty_tracking_start nr_ranges 2 32:[0x0 - 0x807fffff],
>>> 64:[0x100000000 - 0x3838000fffff]
>>>
>>> which seems to be too large for the HW. AFAICT, the MLX5 HW has a 42
>>> bits address space limitation for dirty tracking (min is 12). Is it a
>>> FW tunable or a strict limitation ?
>>
>> It would be good to explain where this is coming from, all devices
>> need to make some decision on what address space ranges to track and I
>> would say 2^42 is already pretty generous limit..
> 
> 
> QEMU computes the DMA logging ranges for two predefined ranges: 32-bit
> and 64-bit. In the OVMF case, QEMU includes in the 64-bit range, RAM
> (at the lower part) and device RAM regions (at the top of the address
> space). The size of that range can be bigger than the 2^42 limit of
> the MLX5 HW for dirty tracking. QEMU is not making much effort to be
> smart. There is room for improvement.
>

Interesting, we haven't reproduced this in our testing with OVMF multi-TB
configs with these VFs. Could you share the OVMF base version you were using? or
maybe we didn't triggered it considering the total device RAM regions would be
small enough to fit the 32G PCI hole64 that is advertised that avoids a
hypothetical relocation.

We could use do more than 2 ranges (or going back to sharing all ranges), or add
a set of ranges that represents the device RAM without computing a min/max there
(not sure we can figure that out from within the memory listener does all this
logic); it would perhaps a bit too BIOS specific if we start looking at specific
parts of the address space (e.g. phys-bits-1) to compute these ranges.

	Joao