privcmd.c not calling set_phys_to_machine

[Stefano Stabellini <sstabellini@kernel.org>]

Hi Juergen and all,

I am writing again to ask a question about privcmd.c in PV dom0 x86.
This is related to the previous pin_user_pages_fast issue:

https://marc.info/?l=xen-devel&m=166268914727630
https://marc.info/?l=xen-devel&m=166322385912052


In summary this is the situation:

1. domU (HVM) kernel space:
    a. pages allocation with get_free_pages()
    b. get dma_handle by calling dma_map_page() on the pages allocated in (1.a)
    c. send dma_handle to dom0 (PV) using virtio queue

2. dom0 userspace (QEMU):
        a. read dma_handle from virtio queue
        b. map dma_handle using QEMU dma_memory_map(), which calls
           xenforeignmemory_map2, which is IOCTL_PRIVCMD_MMAPBATCH_V2,
           which ends up calling drivers/xen/privcmd.c:privcmd_ioctl_mmap_batch
           [this is verified to work correctly, the mapping works]
        c. open /dev/tee node and make an ioctl call to register the
           virtual address (from step 2.b) with TEE.

3. dom0 kernel space:
        a. AMD TEE driver get the virtual address passed by userspace
        b. AMD TEE driver get the list of pages corresponding to the
           virtual address (3.a) and calls dma_map_page() on them

The last step (3.b) misbehaves as dev_addr at the beginning of
xen_swiotlb_map_page (which implements dma_map_page() in dom)) is 0.

  dma_addr_t dev_addr = xen_phys_to_dma(dev, phys);
  /* dev_addr here is zero */


Could it be that the original mapping of the foreign pages in Dom0, done
by step 2.b, is not complete? Looking into
privcmd_ioctl_mmap_batch, for PV guests, it is calling mmap_batch_fn:

	BUG_ON(traverse_pages_block(m.num, sizeof(xen_pfn_t),
				    &pagelist, mmap_batch_fn, &state));

mmap_batch_fn calls xen_remap_domain_gfn_array, which calls
xen_remap_pfn.

xen_remap_pfn only changes the VA->PA mapping and does nothing else.
Specifically, nobody seems to call set_phys_to_machine in this code
path. Isn't set_phys_to_machine required?

Don't we need a call to set_phys_to_machine so that the next time a
driver tries to call:

  /* address is the virtual address passed by QEMU userspace */
  dma_map_page(virt_to_page(address))

it will behave correctly? Or am I missing something?


How is xen_phys_to_dma expected to work correctly for:

  /* address is the virtual address passed by QEMU userspace and mapped
   * in 2.b */
  phys_addr = virt_to_phys(address);
  xen_phys_to_dma(dev, phys_addr);


My guess would be that we need to add:

  set_phys_to_machine(pfn, FOREIGN_FRAME(mfn));

in mmap_batch_fn or xen_remap_pfn?

Thanks for any help or suggestions.

Cheers,

Stefano