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

From: Julien Grall <julien.grall@linaro.org>
To: Jan Beulich <JBeulich@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Andrew Cooper <Andrew.Cooper3@citrix.com>,
	"Tim(Xen.org)" <tim@xen.org>,
	George Dunlap <George.Dunlap@citrix.com>,
	Julien Grall <julien.grall@arm.com>,
	Paul Durrant <Paul.Durrant@citrix.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Ian Jackson <Ian.Jackson@citrix.com>,
	Daniel De Graaf <dgdegra@tycho.nsa.gov>,
	Roger PauMonne <roger.pau@citrix.com>
Subject: Re: [PATCH v12 05/11] x86/mm: add HYPERVISOR_memory_op to acquire guest resources
Date: Thu, 19 Oct 2017 17:06:24 +0100	[thread overview]
Message-ID: <1c3d3841-74bd-3548-1b2b-edf64f9994d8@linaro.org> (raw)
In-Reply-To: <59E8E51B020000780018840F@prv-mh.provo.novell.com>

Hi,

On 19/10/17 16:47, Jan Beulich wrote:
>>>> On 19.10.17 at 17:37, <julien.grall@linaro.org> wrote:
>> Hi,
>>
>> On 19/10/17 16:11, Jan Beulich wrote:
>>>>>> On 19.10.17 at 16:49, <Paul.Durrant@citrix.com> 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.

Cheers,

-- 
Julien Grall

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