Re: [PATCH v4 2/4] xen: Introduce HAS_M2P config and use to protect mfn_to_gmfn call

From: Julien Grall <julien@xen.org>
To: Andrew Cooper <andrew.cooper3@citrix.com>,
	xen-devel@lists.xenproject.org
Cc: "Julien Grall" <julien.grall@arm.com>,
	"Jan Beulich" <jbeulich@suse.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>,
	"George Dunlap" <george.dunlap@citrix.com>,
	"Ian Jackson" <iwj@xenproject.org>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Volodymyr Babchuk" <Volodymyr_Babchuk@epam.com>
Subject: Re: [PATCH v4 2/4] xen: Introduce HAS_M2P config and use to protect mfn_to_gmfn call
Date: Fri, 23 Oct 2020 16:45:57 +0100	[thread overview]
Message-ID: <32d4f762-a61d-bfdd-c4a8-38e5edef1aa8@xen.org> (raw)
In-Reply-To: <ff006b75-73d2-ae21-1811-fbd5c9c244c7@xen.org>

Hi,

On 26/09/2020 14:00, Julien Grall wrote:
> Hi Andrew,
> 
> On 22/09/2020 19:56, Andrew Cooper wrote:
>> On 22/09/2020 19:20, Julien Grall wrote:
>>>>> +
>>>>>    #endif /* __ASM_DOMAIN_H__ */
>>>>>      /*
>>>>> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
>>>>> index 5c5e55ebcb76..7564df5e8374 100644
>>>>> --- a/xen/include/public/domctl.h
>>>>> +++ b/xen/include/public/domctl.h
>>>>> @@ -136,6 +136,12 @@ struct xen_domctl_getdomaininfo {
>>>>>        uint64_aligned_t outstanding_pages;
>>>>>        uint64_aligned_t shr_pages;
>>>>>        uint64_aligned_t paged_pages;
>>>>> +#define XEN_INVALID_SHARED_INFO_FRAME (~(uint64_t)0)
>>>>
>>>> We've already got INVALID_GFN as a constant used in the interface.  
>>>> Lets
>>>> not proliferate more.
>>>
>>> This was my original approach (see [1]) but this was reworked because:
>>>     1) INVALID_GFN is not technically defined in the ABI. So the
>>> toolstack has to hardcode the value in the check.
>>>     2) The value is different between 32-bit and 64-bit Arm as
>>> INVALID_GFN is defined as an unsigned long.
>>>
>>> So providing a new define is the right way to go.
>>
>> There is nothing special about this field.  It should not have a
>> dedicated constant, when a general one is the appropriate one to use.
>>
>> libxl already has LIBXL_INVALID_GFN, which is already used.
> 
> Right, but that's imply it cannot be used by libxc as this would be a 
> layer violation.
> 
>>
>> If this isn't good enough, them the right thing to do is put a proper
>> INVALID_GFN in the tools interface.
> 
> That would be nice but I can see some issue on x86 given that we don't 
> consistenly define a GFN in the interface as a 64-bit value.
> 
> So would you still be happy to consider introducing XEN_INVALID_GFN in 
> the interface with some caveats?

Gentle ping. @Andrew, are you happy with this approach?

Cheers,

-- 
Julien Grall