On 23.08.2021 19:16, Julien Grall wrote: > Hi Jan, > > On 20/08/2021 10:26, Jan Beulich wrote: >> On 20.08.2021 11:08, Julien Grall wrote: >>> On 20/08/2021 08:44, Costin Lupu wrote: >>>> On 8/20/21 9:52 AM, Jan Beulich wrote: >>>>>> --- /dev/null >>>>>> +++ b/xen/include/public/page.h >>>>>> @@ -0,0 +1,36 @@ >>>>>> +/****************************************************************************** >>>>>> + * page.h >>>>>> + * >>>>>> + * Page definitions for accessing guests memory >>>>>> + * >>>>>> + * Permission is hereby granted, free of charge, to any person >>>>>> obtaining a copy >>>>>> + * of this software and associated documentation files (the >>>>>> "Software"), to >>>>>> + * deal in the Software without restriction, including without >>>>>> limitation the >>>>>> + * rights to use, copy, modify, merge, publish, distribute, sublicense, >>>>>> and/or >>>>>> + * sell copies of the Software, and to permit persons to whom the >>>>>> Software is >>>>>> + * furnished to do so, subject to the following conditions: >>>>>> + * >>>>>> + * The above copyright notice and this permission notice shall be >>>>>> included in >>>>>> + * all copies or substantial portions of the Software. >>>>>> + * >>>>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, >>>>>> EXPRESS OR >>>>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >>>>>> MERCHANTABILITY, >>>>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT >>>>>> SHALL THE >>>>>> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR >>>>>> OTHER >>>>>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, >>>>>> ARISING >>>>>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER >>>>>> + * DEALINGS IN THE SOFTWARE. >>>>>> + * >>>>>> + * Copyright (c) 2021, Costin Lupu >>>>>> + */ >>>>>> + >>>>>> +#ifndef __XEN_PUBLIC_PAGE_H__ >>>>>> +#define __XEN_PUBLIC_PAGE_H__ >>>>>> + >>>>>> +#include "xen.h" >>>>>> + >>>>>> +#define XEN_PAGE_SHIFT 12 >>>>>> +#define XEN_PAGE_SIZE (xen_mk_long(1) << XEN_PAGE_SHIFT) >>> >>> This will use UL whereas on Arm a page frame should always be 64-bit >>> regardless the bitness. Shouldn't this be converted to use xen_ulong_t >>> instead? >> >> As pointed out on v1, XEN_PAGE_SIZE would better not end up as a >> value of signed type, for ... > > Did you mean "not end up as a value of **unsigned** type"...
Oh, of course I did. I'm sorry for the confusion caused. >>>>>> +#define XEN_PAGE_MASK (~(XEN_PAGE_SIZE - 1)) >> >> ... this to suitably sign-extend to wider types is necessary. > > ... because, if I am not mistaken, the sign-extension wouldn't happen > with unsigned type. But then on v1 you wrote: > > "Imo the smallest type this should evaluate to is xen_ulong_t" > > Which I interpreted as this value should be 64-bit on Arm32. If this not > what you meant then I am lost. And there I would better have said "If indeed unsigned for whatever reason, the smallest type this should evaluate to is xen_ulong_t." >> Also unless you expect someone to use typeof(XEN_PAGE_SIZE) I'm >> afraid I don't see where the constant being long vs xen_long_t >> (if such existed) might matter. >> Otoh perhaps xen_mk_ulong() would >> better have produced a xen_ulong_t typed values in the first >> place, but I'm afraid we can't alter the existing macro. > > We can create a new one. But we shouldn't carelessly add stuff, as we can't later remove it. Jan
