On 16.09.2024 10:02, Frediano Ziglio wrote: > On Sun, Sep 15, 2024 at 7:16 AM Jan Beulich <jbeul...@suse.com> wrote: >> >> On 10.09.2024 18:16, Frediano Ziglio wrote: >>> --- a/xen/arch/x86/boot/head.S >>> +++ b/xen/arch/x86/boot/head.S >>> @@ -25,6 +25,9 @@ >>> #define MB2_HT(name) (MULTIBOOT2_HEADER_TAG_##name) >>> #define MB2_TT(name) (MULTIBOOT2_TAG_TYPE_##name) >>> >>> +#define BOOT_TYPE_BIOS 1 >>> +#define BOOT_TYPE_PVH 2 >> >> Did you consider using 0 and 1, to be able to use XOR on the BIOS >> path and TEST for checking? >> > > Not clear what you are trying to achieve. Fewer bytes using the XOR? I > think the TEST in this case is only reducing readability, it's an > enumeration.
Except that in practice we don't really need an enum here; a boolean would suffice. > If you are concerns about code size I would use an 8 bit register (I > would say DL) and use EBP register to temporary save EAX, 8 bit > registers have usually tiny instructions, MOV has same size as XOR you > mentioned not loosing any readability or forcing to change values. No, it's not code size alone. It's larger code size for no good reason. Using 8-bit ops is an option when you're truly code size constrained, yet that's not the case here. Hence my take is - use the cheapest insns possible. >>> + cld >> >> So you fold the STDs but not the STIs, despite that not even having >> been first on the PVH path. This decision wants explaining in the >> description, even if just briefly. > > Just in case, I disable interrupts ASAP. Not that this should change > much the result. > Would you prefer to fold it too? That or explain the difference. > By "description" do you mean having an additional comment in the code > or something in the commit message? Sorry, I use "description" as a common synonym for "commit message". >>> @@ -433,14 +451,9 @@ __pvh_start: >>> /* Set up stack. */ >>> lea STACK_SIZE - CPUINFO_sizeof + sym_esi(cpu0_stack), %esp >>> >>> - call initialise_bss >> >> I'm little puzzled: The previous patch moved it "as early as possible" >> just for it to be moved to a later point again here? >> > > The rationale is being able to use C, for that you need a stack, > correct segments and BSS. > Are you suggesting any change? I'm trying to understand the sum effect of both changes, which appear to be moving in opposite direction. Jan
