On 9/16/25 18:13, Mukesh R wrote:
> On 9/15/25 10:55, Michael Kelley wrote:
>> From: Mukesh Rathor <[email protected]> Sent: Tuesday, September
>> 9, 2025 5:10 PM
>>>
>>> Introduce a new file to implement collection of hypervisor ram into the
>>
>> s/ram/RAM/ (multiple places)
>
> a quick grep indicates using saying ram is common, i like ram over RAM
>
>>> vmcore collected by linux. By default, the hypervisor ram is locked, ie,
>>> protected via hw page table. Hyper-V implements a disable hypercall which
>>
>> The terminology here is a bit confusing since you have two names for
>> the same thing: "disable" hypervisor, and "devirtualize". Is it possible to
>> just use "devirtualize" everywhere, and drop the "disable" terminology?
>
> The concept is devirtualize and the actual hypercall was originally named
> disable. so intermixing is natural imo.
[snip]
>>> +
>>> +/*
>>> + * Setup a temporary gdt to allow the asm code to switch to the long mode.
>>> + * Since the asm code is relocated/copied to a below 4G page, it cannot
>>> use rip
>>> + * relative addressing, hence we must use trampoline_pa here. Also, save
>>> other
>>> + * info like jmp and C entry targets for same reasons.
>>> + *
>>> + * Returns: 0 on success, -1 on error
>>> + */
>>> +static int hv_crash_setup_trampdata(u64 trampoline_va)
>>> +{
>>> + int size, offs;
>>> + void *dest;
>>> + struct hv_crash_tramp_data *tramp;
>>> +
>>> + /* These must match exactly the ones in the corresponding asm file */
>>> + BUILD_BUG_ON(offsetof(struct hv_crash_tramp_data, tramp32_cr3) != 0);
>>> + BUILD_BUG_ON(offsetof(struct hv_crash_tramp_data, kernel_cr3) != 8);
>>> + BUILD_BUG_ON(offsetof(struct hv_crash_tramp_data, gdtr32.limit) != 18);
>>> + BUILD_BUG_ON(offsetof(struct hv_crash_tramp_data,
>>> + cs_jmptgt.address) != 40);
>>
>> It would be nice to pick up the constants from a #include file that is
>> shared with the asm code in Patch 4 of the series.
>
> yeah, could go either way, some don't like tiny headers... if there are
> no objections to new header for this, i could go that way too.
yeah, i experimented with creating a new header or try to add to existing.
new header doesn't make sense for just 5 #defines, adding C struct there
is not a great idea given it's scope is limited to the specific function
in the c file. adding to another header results in ifdefs for ASM/KERNEL,
so not really worth it. I think for now it is ok, we can live with it.
If arm ends up adding more declarations, we can look into it.
Thanks,
-Mukesh
[ .. deleted.. ]
