Re: [PATCH] public: xen: Define missing guest handle for int32_t

[Jan Beulich]

On 26.04.2024 00:39, Stefano Stabellini wrote:
> On Mon, 22 Apr 2024, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 17/04/2024 19:49, Stefano Stabellini wrote:
>>> On Wed, 17 Apr 2024, Julien Grall wrote:
 Hi Michal,

 On 17/04/2024 13:14, Michal Orzel wrote:
> Commit afab29d0882f ("public: s/int/int32_t") replaced int with int32_t
> in XEN_GUEST_HANDLE() in memory.h but there is no guest handle defined
> for it. This results in a build failure. Example on Arm:
>
> ./include/public/arch-arm.h:205:41: error: unknown type name
> ‘__guest_handle_64_int32_t’
> 205 | #define __XEN_GUEST_HANDLE(name)__guest_handle_64_ ##
> name
> | ^~
> ./include/public/arch-arm.h:206:41: note: in expansion of macro
> ‘__XEN_GUEST_HANDLE’
> 206 | #define XEN_GUEST_HANDLE(name)
> __XEN_GUEST_HANDLE(name)
> | ^~
> ./include/public/memory.h:277:5: note: in expansion of macro
> ‘XEN_GUEST_HANDLE’
> 277 | XEN_GUEST_HANDLE(int32_t) errs;
>
> Fix it. Also, drop guest handle definition for int given no further use.
>
> Fixes: afab29d0882f ("public: s/int/int32_t")
> Signed-off-by: Michal Orzel 
>>>
>>> Reviewed-by: Stefano Stabellini 
>>>
>>>
 So it turned out that I committed v1 from Stefano. I was meant to commit
 the
 patch at all, but I think I started with a dirty staging :(. Sorry for
 that.

 I have reverted Stefano's commit for now so we can take the correct patch.

 Now, from my understanding, Andrew suggested on Matrix that this solution
 may
 actually be a good way to handle GUEST_HANLDEs (they were removed in v2).
 Maybe this can be folded in Stefano's patch?
>>>
>>> v1 together with Michal's fix is correct. Also v2 alone is correct, or
>>> v2 with Michal's fix is also correct.
>>
>> I am slightly confused, v2 + Michal's fix means that XEN_GUEST_HANDLE(int) is
>> removed and we introduce XEN_GUEST_INT(int32_t) with no user. So wouldn't 
>> this
> 
> You are right I apologize. I looked at Michal's patch too quickly and
> I thought it was just adding XEN_GUEST_INT(int32_t) without removing
> anything.
> 
> In that case, if you are OK with it, please ack and commit v2 only.

Just to mention it: Committing would apparently be premature, as I can't spot
any response to comments I gave to the patch. I'm okay with those being
addressed verbally only, but imo they cannot be dropped on the floor.

Jan

Re: [PATCH v2 1/1] xen/arm64: entry: Add missing code symbol annotations

[Jan Beulich]

On 26.04.2024 01:13, Stefano Stabellini wrote:
> On Tue, 16 Apr 2024, Edgar E. Iglesias wrote:
>> From: "Edgar E. Iglesias" 
>>
>> Use the generic xen/linkage.h macros when and add missing
> ^ when what?
> 
>> code symbol annotations.
>>
>> Signed-off-by: Edgar E. Iglesias 
> 
> I am looking at the implementation of FUNC and as far as I can tell
> there is no change compared to ENTRY. So from that point of view we are
> good. I wonder if we should keep using "ENTRY" because it is nice to
> mark explicitely the entry points as such but at the same time I am also
> OK with this. I'll let the other ARM maintainers decide.

Just to mention it: ENTRY should go away (and hence why PPC and RISC-V had
it dropped already, while x86 has patches pending to reduce its scope
enough), not the least to finally allow the oddity near the top of xen.lds.S
to go away.

Jan