Re: [RFC PATCH 15/45] KVM: arm64: pkvm: Add __pkvm_host_share/unshare_dma()

tina.zhang
2023-02-07T02:39:24Z

Hi Jean,

On 2/6/23 20:13, Jean-Philippe Brucker wrote:
> Hi Tina,
>
> On Sat, Feb 04, 2023 at 08:51:38PM +0800, tina.zhang wrote:
>>> +int __pkvm_host_share_dma(phys_addr_t phys_addr, size_t size, bool is_ram)
>>> +{
>>> + int i;
>>> + int ret;
>>> + size_t nr_pages = size >> PAGE_SHIFT;
>>> +
>>> + if (WARN_ON(!PAGE_ALIGNED(phys_addr | size)))
>>> + return -EINVAL;
>>> +
>>> + host_lock_component();
>>> + hyp_lock_component();
>>> +
>>> + for (i = 0; i < nr_pages; i++) {
>>> + ret = __pkvm_host_share_dma_page(phys_addr + i * PAGE_SIZE,
>>> + is_ram);
>> Hi Jean,
>>
>> I'm not familiar with ARM arch. Just out of curiosity. If pKVM-ARM populates
>> the host stage-2 page table lazily, would there be a case that device driver
>> in host triggers DMA with pages which have not been mapped to the host
>> stage-2 page table yet? How do we handle this situation?
>
> It's possible that the host asks the hypervisor to map on the IOMMU side
> a page that is not yet mapped on the CPU side. In general before calling
> map_pages() the host zero-initializes the page, triggering a page fault
> which creates the mapping, so this case is rare. But if it happens,
> __pkvm_host_share_dma() will create the CPU stage-2 mapping:
>
> __pkvm_host_share_dma()
> do_share()
> host_initiate_share()
> host_stage2_idmap_locked()
>
> which creates a valid identity mapping, along with the ownership
> information PVKM_PAGE_SHARED_OWNED. That ownership info is really all we
> need here, to prevent future donations to guests or hyp. Since the SMMU
> side uses separate stage-2 page tables, we don't actually need to create a
> valid mapping on the CPU side yet, that's just how pKVM's mem_protect
> currently works. But I don't think it hurts to create the mapping right
> away instead of waiting for the CPU page fault, because the host will
> likely access the page soon to read what the device wrote there.
Right. I was thinking if stage-2 lazy mapping method is being adopted,
whether there would be a case that the check_share() couldn't pass, as
the pte might not be valid at that time. After checking the logic, I see
we check kvm_pte_valid(pte) as well as the value of pte. So I guess the
check_share() can return successfully even w/o having triggered host CPU
page fault. Thanks for the elaborating.

Regards,
-Tina

>
> Thanks,
> Jean
>