Re: [PATCH] x86/hvm: Widen condition for is_hvm_pv_evtchn_vcpu()

[Jane Malalane <Jane.Malalane@citrix.com>]

On 18/05/2022 10:07, Jan Beulich wrote:
> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
> 
> On 11.05.2022 17:14, Jane Malalane wrote:
>> Have is_hvm_pv_evtchn_vcpu() return true for vector callbacks for
>> evtchn delivery set up on a per-vCPU basis via
>> HVMOP_set_evtchn_upcall_vector.
> 
> I'm confused: You say "per-vCPU" here, but ...
> 
>> --- a/xen/arch/x86/include/asm/domain.h
>> +++ b/xen/arch/x86/include/asm/domain.h
>> @@ -14,8 +14,14 @@
>>   
>>   #define has_32bit_shinfo(d)    ((d)->arch.has_32bit_shinfo)
>>   
>> +/*
>> + * Set to true if either the global vector-type callback or per-vCPU
>> + * LAPIC vectors are used. Assume all vCPUs will use
>> + * HVMOP_set_evtchn_upcall_vector as long as the initial vCPU does.
>> + */
>>   #define is_hvm_pv_evtchn_domain(d) (is_hvm_domain(d) && \
>> -        (d)->arch.hvm.irq->callback_via_type == HVMIRQ_callback_vector)
>> +        ((d)->arch.hvm.irq->callback_via_type == HVMIRQ_callback_vector || \
>> +         (d)->vcpu[0]->arch.hvm.evtchn_upcall_vector))
> 
> ... you use (d)->vcpu[0] here (and, yes, you say so in the comment)
> and ...
> 
>>   #define is_hvm_pv_evtchn_vcpu(v) (is_hvm_pv_evtchn_domain(v->domain))
> 
> ... you don't alter this at all.
> 
> Also (re-ordering context) this ...
> 
>> is_hvm_pv_evtchn_vcpu() returning true is a condition for setting up
>> physical IRQ to event channel mappings.
> 
> ... isn't really true - it's is_hvm_pv_evtchn_domain() which controls
> this (which in turn is why above you need to make the assumption I've
> put under question). With that assumption I think is_hvm_pv_evtchn_vcpu()
> would better go away.
Here only is_hvm_pv_evtchn_domain() should have been mentioned. The 
"per-VCPU" was in regard to the vector callback for evthcn delivery 
setup not being global but now done on a per-vCPU basis, in any case, I 
will amend the description and title. Thanks for the feedback.

Jane.