On Thu, Jul 13, 2023 at 11:51 PM Christopher Clark < [email protected]> wrote:
> > > On Sat, Jul 8, 2023 at 11:47 AM Stefano Stabellini <[email protected]> > wrote: > >> On Sat, 1 Jul 2023, Christopher Clark wrote: >> > To convert the x86 boot logic from multiboot to boot module structures, >> > change the bootstrap map function to accept a boot module parameter. >> > >> > To allow incremental change from multiboot to boot modules across all >> > x86 setup logic, provide a temporary inline wrapper that still accepts a >> > multiboot module parameter and use it where necessary. The wrapper is >> > placed in a new arch/x86 header <asm/boot.h> to avoid putting a static >> > inline function into an existing header that has no such functions >> > already. This new header will be expanded with additional functions in >> > subsequent patches in this series. >> > >> > No functional change intended. >> > >> > Signed-off-by: Christopher Clark <[email protected]> >> > Signed-off-by: Daniel P. Smith <[email protected]> >> > >> >> [...] >> >> > diff --git a/xen/include/xen/bootinfo.h b/xen/include/xen/bootinfo.h >> > index b72ae31a66..eb93cc3439 100644 >> > --- a/xen/include/xen/bootinfo.h >> > +++ b/xen/include/xen/bootinfo.h >> > @@ -10,6 +10,9 @@ >> > #endif >> > >> > struct boot_module { >> > + paddr_t start; >> > + size_t size; >> >> I think size should be paddr_t (instead of size_t) to make sure it is >> the right size on both 64-bit and 32-bit architectures that support >> 64-bit addresses. >> > > Thanks, that explanation does make sense - ack. > I've come back to reconsider this as it doesn't seem right to me to store a non-address value (which this will always be) in a type explicitly defined to hold an address: addresses may have architectural alignment requirements whereas a size value is just a number of bytes so will not. The point of a size_t value is that size_t is defined to be large enough to hold the size of any valid object in memory, so I think this was right as-is. Christopher > > Christopher > > >> >> >> > struct arch_bootmodule *arch; >> > }; >> >
