[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] tools/xen-foreign: Update the size for vcpu_guest_{core_regs, context}
Hi Jan,
On 06/07/2021 16:10, Jan Beulich wrote:
On 06.07.2021 16:24, Julien Grall wrote:
On 06/07/2021 15:07, Jan Beulich wrote:
On 06.07.2021 15:20, Julien Grall wrote:
From: Julien Grall <jgrall@xxxxxxxxxx>
Commit 918b8842a852 ("arm64: Change type of hsr, cpsr, spsr_el1 to
uint64_t") updated the size of the structure vcpu_guest_core_regs and
indirectly vcpu_guest_context.
On Arm, the two structures are only accessible to the tools and the
hypervisor (and therefore stable). However, they are still checked
I suppose s/stable/not stable/ and ...
by the scripts in tools/include/xen-foreign are not able to understand
that.
Ideally we should rework the scripts so we don't have to update
the size for non-stable structure. But I don't have limited time
... s/don't/only/ ?
Yes I will update.
to spend on the issue. So chose the simple solution and update
the size accordingly.
Note that we need to keep vcpu_guest_core_regs around because
the structure is used by vcpu_guest_context and therefore the
scripts expects the generated header to contain it.
If vcpu_guest_context is also tools-only, why does it need keeping
(for Arm)? IOW can't you drop vcpu_guest_core_regs from the scripts
altogether, and mark vcpu_guest_context with valid sizes for x86
only?
I have tried to use "-" in place of the size but the checker is still
not happy. I didn't find another way without modifying the script...
I don't know the script and I don't have a lot of time to work on the
bug fix. So this is not something I could do this week.
So this leave us to the following options:
1) Someone volunteer to fix the script
2) Revert the patch that broke the build
3) Accept this patch
I would prefer 3) so we have more time to work on making the script a
lot smarter.
Well, if we have no-one who knows the script, and the simple thing
of removing the one obvious line doesn't help, then I agree we want
to go with 3) for now, even if it looks odd to adjust number for
something that's not supposed to be checked in the first place. Of
course I also wouldn't mind 2), as it seems pretty clear that the
patch was insufficiently build-tested.
The series has dragged on for quite a while on the ML. So I would like
to avoid a revert and wait for further rework in the tools side.
Also Bertrand pointed out there is a second fix necessary (I think they
should still live separately). I will wait him to confirm that the
toolstack is building for 32-bit and 64-bit after the two patches are
applied.
Cheers,
--
Julien Grall
|
