On 5/25/2020 12:29 PM, Yan Zhao wrote:
On Tue, May 19, 2020 at 10:58:04AM -0600, Alex Williamson wrote:
Hi folks,
My impression is that we're getting pretty close to a workable
implementation here with v22 plus respins of patches 5, 6, and 8. We
also have a matching QEMU series and a proposal for a new i40e
consumer, as well as I assume GVT-g updates happening internally at
Intel. I expect all of the latter needs further review and discussion,
but we should be at the point where we can validate these proposed
kernel interfaces. Therefore I'd like to make a call for reviews so
that we can get this wrapped up for the v5.8 merge window. I know
Connie has some outstanding documentation comments and I'd like to make
sure everyone has an opportunity to check that their comments have been
addressed and we don't discover any new blocking issues. Please send
your Acked-by/Reviewed-by/Tested-by tags if you're satisfied with this
interface and implementation. Thanks!
hi Alex
after porting gvt/i40e vf migration code to kernel/qemu v23, we spoted
two bugs.
1. "Failed to get dirty bitmap for iova: 0xfe011000 size: 0x3fb0 err: 22"
This is a qemu bug that the dirty bitmap query range is not the same
as the dma map range. It can be fixed in qemu. and I just have a little
concern for kernel to have this restriction.
I never saw this unaligned size in my testing. In this case if you can
provide vfio_* event traces, that will helpful.
2. migration abortion, reporting
"qemu-system-x86_64-lm: vfio_load_state: Error allocating buffer
qemu-system-x86_64-lm: error while loading state section id 49(vfio)
qemu-system-x86_64-lm: load of migration failed: Cannot allocate memory"
It's still a qemu bug and we can fixed it by
"
if (migration->pending_bytes == 0) {
+ qemu_put_be64(f, 0);
+ qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
"
In which function in QEMU do you have to add this?
and actually there are some extra concerns about this part, as reported in
[1][2].
[1] data_size should be read ahead of data_offset
https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg02795.html.
[2] should not repeatedly update pending_bytes in vfio_save_iterate()
https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg02796.html.
but as those errors are all in qemu, and we have finished basic tests in
both gvt & i40e, we're fine with the kernel part interface in general now.
(except for my concern [1], which needs to update kernel patch 1)
>> what if pending_bytes is not 0, but vendor driver just does not want to
>> send data in this iteration? isn't it right to get data_size first
before
>> getting data_offset?
If vendor driver doesn't want to send data but still has data in staging
buffer, vendor driver still can control to send pending_bytes for this
iteration as 0 as this is a trap field.
I would defer this to Alex.
so I wonder which way in your mind is better, to give our reviewed-by to
the kernel part now, or hold until next qemu fixes?
and as performance data from gvt is requested from your previous mail, is
that still required before the code is accepted?
BTW, we have also conducted some basic tests when viommu is on, and found out
errors like
"qemu-system-x86_64-dt: vtd_iova_to_slpte: detected slpte permission error (iova=0x0, level=0x3, slpte=0x0, write=1)
qemu-system-x86_64-dt: vtd_iommu_translate: detected translation failure (dev=00:03:00, iova=0x0)
qemu-system-x86_64-dt: New fault is not recorded due to compression of faults".
I saw these errors, I'm looking into it.
Thanks,
Kirti
