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 <jbeul...@suse.com>
>
> Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>,
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
