Re: [PATCH v2 03/26] xen/x86: consolidate vram tracking support

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.09.2025 09:54, Penny, Zheng wrote:
>> -----Original Message-----
>> From: Penny, Zheng
>> Sent: Thursday, September 11, 2025 3:16 PM
>>
>>> -----Original Message-----
>>> From: Jan Beulich <jbeulich@xxxxxxxx>
>>> Sent: Wednesday, September 10, 2025 10:09 PM
>>>
>>> On 10.09.2025 09:38, Penny Zheng wrote:
>>>> --- a/xen/arch/x86/include/asm/paging.h
>>>> +++ b/xen/arch/x86/include/asm/paging.h
>>>> @@ -133,13 +133,20 @@ struct paging_mode {
>>>>      (DIV_ROUND_UP(PADDR_BITS - PAGE_SHIFT - (PAGE_SHIFT + 3), \
>>>>                    PAGE_SHIFT - ilog2(sizeof(mfn_t))) + 1)
>>>>
>>>> -#if PG_log_dirty
>>>> +#ifdef CONFIG_HVM
>>>> +/* VRAM dirty tracking support */
>>>> +struct sh_dirty_vram {
>>>> +    unsigned long begin_pfn;
>>>> +    unsigned long end_pfn;
>>>> +#ifdef CONFIG_SHADOW_PAGING
>>>> +    paddr_t *sl1ma;
>>>> +    uint8_t *dirty_bitmap;
>>>> +    s_time_t last_dirty;
>>>> +#endif
>>>> +};
>>>> +#endif
>>>
>>> Subsequently I think we will want to do more cleanup here. Us using a
>>> shadow mode struct also in HAP code is bogus and, afaics, wasteful.
>>> The three latter members are used only by shadow code, so HAP could
>>> have its own, smaller variant of the type. And each type could be
>>> private to the hap/ and shadow/ subtrees respectively.
>>
>> Understood.
> 
> Reading relative codes, found that we have a "struct sh_dirty_vram 
> *dirty_vram" in "struct hvm_domain",
> If we defined different type "struct hap_dirty_vram" and "struct 
> sh_dirty_vram" private to the hap/ and shadow/ subtrees respectively, either 
> we add different type in "struct hvm_domain", or we change it to the "void *" 
> there and do the type casting on referring... maybe the former is safer or 
> any better suggestion?

Yes, but I wasn't really meaning for you to do that further cleanup. I'm
intending to do that once your change has gone in.

Jan