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

[tina.zhang]

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
>