On 7/18/22 09:58, Smith, Jackson wrote:
> Hi Daniel,
>
> I hope outlook gets this reply right.
Looks good to me, thank you for taking the time to review.
>> -Original Message-
>> Subject: [PATCH v1 04/18] x86: refactor entrypoints to new boot info
>
>> diff --git a/xen/arch/x86/guest/xen/pvh-boot.c
>> b/xen/arch/x86/guest/xen/pvh-boot.c
>> index 834b1ad16b..28cf5df0a3 100644
>> --- a/xen/arch/x86/guest/xen/pvh-boot.c
>> +++ b/xen/arch/x86/guest/xen/pvh-boot.c
>
>> @@ -99,13 +118,16 @@ static void __init get_memory_map(void)
>> sanitize_e820_map(e820_raw.map, _raw.nr_map);
>> }
>>
>> -void __init pvh_init(multiboot_info_t **mbi, module_t **mod)
>> +void __init pvh_init(struct boot_info **bi)
>> {
>> -convert_pvh_info(mbi, mod);
>> +*bi = init_pvh_info();
>> +convert_pvh_info(*bi);
>>
>> hypervisor_probe();
>> ASSERT(xen_guest);
>>
>> +(*bi)->arch->xen_guest = xen_guest;
>
> I think you may have a typo/missed refactoring here?
> I changed this line to "(*bi)->arch->xenguest = xen_guest;" to get the
> patchset to build.
Hmm, I guess I missed one. I originally was going to mimic the name
xen_guest in the structure definition but when xen guest support is
disable the xen_guest global turns into a #define which replaces the
reference resulting in a compilation error.
> The arch_boot_info struct in boot_info32.h has a field 'xen_guest' but the
> same field in asm/bootinfo.h was re-named from 'xen_guest' to 'xenguest' in
> the 'x86: adopt new boot info structures' commit.
>
> What was your intent?
As I mentioned above, the renaming was intentional, and it looks like I
do a poor job catching everywhere where the renaming need to be done.
>> +
>> get_memory_map();
>> }
>>
>
> Thanks,
> Jackson Smith
v/r,
dps
