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

[Jean-Philippe Brucker <jean-philippe@linaro.org>]

On Tue, Feb 07, 2023 at 10:37:51AM +0800, tina.zhang wrote:
> 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.

Yes, during check_share(), host_get_page_state() allows disabled PTE
unless the PTE already contains ownership information (in particular, if
the page has been donated to a guest or to the hypervisor, the PTE is
invalid and contains the non-zero owner ID.)

Thanks,
Jean