Re: [PATCH 7/7] x86/HVM: drop stdvga's "vram_page[]" struct member

[Jan Beulich <jbeulich@xxxxxxxx>]

On 10.09.2024 20:28, Andrew Cooper wrote:
> On 10/09/2024 3:41 pm, Jan Beulich wrote:
>> No uses are left, hence its setup, teardown, and the field itself can
>> also go away. stdvga_deinit() is then empty and can be dropped as well.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>,

Thanks.

> although I think there's more to do.
> 
>> --- a/xen/arch/x86/include/asm/hvm/io.h
>> +++ b/xen/arch/x86/include/asm/hvm/io.h
>> @@ -111,12 +111,10 @@ struct vpci_arch_msix_entry {
>>  };
>>  
>>  struct hvm_hw_stdvga {
>> -    struct page_info *vram_page[64];  /* shadow of 0xa0000-0xaffff */
>>      spinlock_t lock;
>>  };
> 
> I'm pretty sure you can drop the lock too.  It's taken in accept(), and
> dropped in complete(), but there's no state at all to be protected.
> 
> stdvga_mem_accept()'s return value is a simple expression of p.

I think you're right. Previously I was assuming the lock was (also) about
serializing the bufioreq sending, yet ioreq_send_buffered() has its own
serialization. And hence yes, with all other state gone, the lock can go
too, as can ...

> With that dropped, the complete() handler disappears, and it's the only
> hvm_io_ops->complete() handler in Xen so the whole field can go.

... this hook.

> So I'm pretty sure there are 2 more patches that ought to be part of
> this series, which go in a further negative direction.

Will do.

Jan