Re: [Xen-devel] [PATCH v12 05/11] x86/mm: add HYPERVISOR_memory_op to acquire guest resources

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi,

On 19/10/17 17:06, Julien Grall wrote:
> On 19/10/17 16:47, Jan Beulich wrote:
> > > > > On 19.10.17 at 17:37, <julien.grall@xxxxxxxxxx> wrote:
> > > Hi,
> > >
> > > On 19/10/17 16:11, Jan Beulich wrote:
> > > > > > > On 19.10.17 at 16:49, <Paul.Durrant@xxxxxxxxxx> wrote:
> > > > > > > I'd prefer to make the whole thing x86-only since that's the only 
> > > > > > > platform
> > > > > > on which I can test it, and indeed the code used to be x86-only. Jan
> > > objected
> > > > > > to this so all I'm trying to achieve is that it builds for ARM. 
> > > > > > Please can
> > > you and
> > > > > > Jan reach agreement on where the code should live and how, if at 
> > > > > > all, it
> > > > > > should be #ifdef-ed?
> > > > > >
> > > > > > I am quite surprised of "it is tools-only" so it is fine to not 
> > > > > > protect
> > > > > > it even if it is x86 only. That's probably going to bite us in the 
> > > > > > future.
> > > > > So, this appears to have reached an impasse. I don't know how to 
> > > > > proceed
> > > > > without having to also fix priv mapping for x86, which is a yak far 
> > > > > too
> > > large
> > > > > for me to shave at the moment.
> > > > Julien,
> > > >
> > > > why is it that you are making refcounting on p2m insertion / removal
> > > > a requirement for this series? We all know it's not there right now
> > > > (and similarly also not for the IOMMU, which might affect ARM as well
> > > > unless you always use shared page tables), and it's - as Paul validly
> > > > says - unrelated to his series.
> > > Well, we do at least have refcounting for foreign mapping on Arm. So if
> > > a foreign domain remove the mapping, the page will stay allocated until
> > > the last mapping is removed. For IOMMU, page tables are for the moment
> > > always shared.
> > >
> > > If you don't want to fix x86 now, then it is fine. But I would
> > > appreciate if you don't spread that on Arm.
> > >
> > > To give you a bit more context, I was ready to implement the Arm version
> > > of set_foreign_p2m_entry (it is quite trivial) to append at the end of
> > > this series. But given that refcounting is not done, I am more reluctant
> > > to do that.
> > I don't understand: The refcounting is to be done by ARM-specific
> > code anyway, i.e. by the implementation of set_foreign_p2m_entry(),
> > not its caller. At least that's what I would have expected.
> I thought I said it before, but it looks like not. Assuming the MFN is 
> always baked by a domain, the prototype would likely need to be extended 
> and take the foreign domain.
> If it is not the case, we would need to find another way to do refcounting.
Looking a bit more at the resource you can acquire from this hypercall. 
Some of them are allocated using alloc_xenheap_page() so not assigned to 
a domain.
So I am not sure how you can expect a function set_foreign_p2m_entry to 
take reference in that case.
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel