Re: [PATCH 2/3] xen/arm, tools: Add a new HVM_PARAM_MAGIC_BASE_PFN key in HVMOP

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.04.2024 10:12, Henry Wang wrote:
> Hi Jan,
> 
> On 4/30/2024 2:11 PM, Jan Beulich wrote:
>> On 30.04.2024 04:51, Henry Wang wrote:
>>> On 4/30/2024 8:31 AM, Daniel P. Smith wrote:
>>>> On 4/26/24 02:21, Jan Beulich wrote:
>>>>> On 26.04.2024 05:14, Henry Wang wrote:
>>>>>> --- a/xen/include/public/hvm/params.h
>>>>>> +++ b/xen/include/public/hvm/params.h
>>>>>> @@ -76,6 +76,7 @@
>>>>>>     */
>>>>>>    #define HVM_PARAM_STORE_PFN    1
>>>>>>    #define HVM_PARAM_STORE_EVTCHN 2
>>>>>> +#define HVM_PARAM_MAGIC_BASE_PFN    3
>>>>>>      #define HVM_PARAM_IOREQ_PFN    5
>>>>> Considering all adjacent values are used, it is overwhelmingly likely
>>>>> that
>>>>> 3 was once used, too. Such re-use needs to be done carefully. Since you
>>>>> need this for Arm only, that's likely okay, but doesn't go without (a)
>>>>> saying and (b) considering the possible future case of dom0less becoming
>>>>> arch-agnostic, or hyperlaunch wanting to extend the scope. Plus (c) imo
>>>>> this also needs at least a comment, maybe even an #ifdef, seeing how
>>>>> x86-
>>>>> focused most of the rest of this header is.
>>>> I would recommend having two new params,
>>> Sounds good. I can do the suggestion in v2.
>>>
>>>> #define HVM_PARAM_HV_RSRV_BASE_PVH 3
>>>> #define HVM_PARAM_HV_RSRV_SIZE 4
>>> I think 4 is currently in use, so I think I will find another couple of
>>> numbers in the end for both of them. Instead of reusing 3 and 4.
>> Right. There are ample gaps, but any use of values within a gap will need
>> appropriate care. FTAOD using such a gap looks indeed preferable, to avoid
>> further growing the (sparse) array. Alternatively, if we're firm on this
>> never going to be used on x86, some clearly x86-specific indexes (e.g. 36
>> and 37) could be given non-x86 purpose.
> 
> Sorry, I am a bit confused. I take Daniel's comment as to add two new 
> params, which is currently only used for Arm, but eventually will be 
> used for hyperlaunch on x86 (as the name indicated). So I think I will 
> use the name that he suggested, but the number changed to 39 and 40.

Well, yes, if re-use for x86 is intended, then unused slots need taking.
Question then still is whether the array bounds indeed need moving up,
or whether instead one of the gaps can be (re)used.

Jan