[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Roger Pau Monne <roger.pau@xxxxxxxxxx>
  • From: Jane Malalane <Jane.Malalane@xxxxxxxxxx>
  • Date: Mon, 16 May 2022 12:22:44 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=wIAnczFkxUvcdgGB2qZTtqomp2OnmE37nOteaSXjqzQ=; b=OgTpYseozli+ZFkFz5g9ZBn28L3aH2x1fDtWYsIkJ1YlJHICyqR5qGUpnVKIDhPWh7MBPxhTd+zdpVD3JYOahmZCzfJzOlj9Db4UxBFLP2zWsrLeQ6FdMCNVAPo5x717bITD6zRAPPtmX0Sf96N1EYYmqt8iWcg55HGVjfYGW3JgpTlrhlmdfuhENXcesKJMPZNg4sdZ5i7CiXGcLotH2ZHHcyX35Dm85p96jUBs4MuRwHFvfrRZdIIqfN47P6luBMN4FveA7pVPLzfl2U6WQa5CLCV763a920nABag7KYFhEzdANKee6IUNuSCeyxm3ppFDEAGCDFtgibev91rzQg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=JlbBPoH3vgymmT/cdeUmshMZ7PS/bkOu00vt4AeJ+hgkvxFJOi2WtYSWikunOf7bs24HZ4LicHlyigaGc2euJIHeBVhbVdYDTXqSMtyVEso0HN36BrX2hyvqK96h0CJlb5u5pAaH4rLKKgUWA7Rt1qEVYP+AejYX86eBOEqcgnE96vA6qghI2Ds8sl7d4fnapZCF4TW6uqeRe72HzZ3N9UR1v6L49G43CSu78UyX0k4IPtSTwwSoFlItKciJbTvZPilT0cDyF+rbVA9lzu3IxevtHdU4ASKfAo7+YUHDhcUE0j8YohIw0c89bCefn5ZeT9d8nd3iZnAIc0Nh/apfJA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Mon, 16 May 2022 12:23:21 +0000
  • Ironport-data: A9a23:UEYSqKDC4o66khVW/1riw5YqxClBgxIJ4kV8jS/XYbTApDwk1GcDy mJJUG2BP/aNa2T1Koskb4q+8k5XsZDUmtNlQQY4rX1jcSlH+JHPbTi7wuYcHM8wwunrFh8PA xA2M4GYRCwMZiaA4E/raNANlFEkvU2ybuOU5NXsZ2YgHGeIdA970Ug5w7Ng39Yy6TSEK1jlV e3a8pW31GCNg1aYAkpMg05UgEoy1BhakGpwUm0WPZinjneH/5UmJMt3yZWKB2n5WuFp8tuSH I4v+l0bElTxpH/BAvv9+lryn9ZjrrT6ZWBigVIOM0Sub4QrSoXfHc/XOdJFAXq7hQllkPhe2 pZP9sHvRzx3Affzo78bXDJFHA1XaPguFL/veRBTsOS15mieLz7J/K8rC0s7e4oF5uxwHGdCs +QCLywAZQyCgOTwx6+nTu5rhYIoK8yD0IE34yk8i22GS6l9B8mcGs0m5vcBtNs0ruJHG/uYQ sMdYD5mahnoaBxTIFYHTpk5mY9Eg1GgKmIJ9ArI+cLb5UDJkClg3+bvM+GNa9+WdcNvtECAq jrZqjGR7hYycYb3JSC+2mKhgKrDkD32XKoWFaak7bh6jVuL3GsRBRYKE1yhrpGRiESzRtZeI Ew84Tc1oO4580nDZsb5dw21pjiDpBF0ZjZLO+gz6QXIz7WO5Q+cXjgAVmQYN4Vgs9IqTzs30 FPPh8nuGTFkrLySTzSa66uQqjSxfyMSKAfueBM5cOfM2PG7yKlbs/4FZo0yeEJpprUZwQ3N/ g0=
  • Ironport-hdrordr: A9a23:kohQ2qhCBMTZnMH6NDyRRDLWbXBQX3h13DAbv31ZSRFFG/FwyP rCoB1L73XJYWgqM03IwerwQ5VpQRvnhP1ICRF4B8buYOCUghrTEGgE1/qv/9SAIVy1ygc578 tdmsdFebrN5DRB7PoSpTPIa+rIo+P3v5xA592uqUuFJDsCA84P0+46MHfjLqQcfnglOXNNLu v52iMxnUvERZ14VKSGL0hAe9KGi8zAlZrgbxJDLQUg8hOygTSh76O/OwSE3z8FOgk/gIsKwC zgqUjU96+ju/a0xlv3zGnI9albn9Pn159qGNGMsM4IMT/h4zzYJLiJGofy/wzdktvfrWrCo+ O85yvI+P4DrE85S1vF4ycFHTOQlgrGpUWSkGNwykGT3PARDAhKd/apw7gpPCcxonBQw+1Uwe ZF2XmUuIFQCg6FlCPh58LQXxUvjUasp2E++NRjxkC3fLFuH4O5l7Zvin+90a1wbx7S+cQiCq 1jHcvc7PFZfReTaG3YpHBmxJipUm4oFhmLT0AesojNugIm10xR3g8d3ogSj30A/JUyR91N4P nFKL1hkPVLQtUNZaxwCe8dSY+8C3DLQxjLLGWOSG6XXJ0vKjbIsdr68b817OaldNgBy4Yzgo 3IVBdCuWs7ayvVeLmzNV1wg2XwqUmGLEXQI5tlluZEU5XHNcrWGDzGTkwymM29pPhaCtHHWp +ISeBrP8M=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHYZUnyc7oF7QLoj0+gbrrX4WFU9K0c9NIAgAR/4IA=
  • Thread-topic: [PATCH] x86/hvm: Widen condition for is_hvm_pv_evtchn_vcpu()

On 13/05/2022 16:39, Roger Pau Monné wrote:
> On Wed, May 11, 2022 at 04:14:23PM +0100, 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.
>>
>> is_hvm_pv_evtchn_vcpu() returning true is a condition for setting up
>> physical IRQ to event channel mappings.
> 
> I would add something like:
> 
> The naming of the CPUID bit is a bit generic about upcall support
> being available.  That's done so that the define name doesn't get
> overly long like XEN_HVM_CPUID_UPCALL_VECTOR_SUPPORTS_PIRQ or some
> such.
> 
> Guests that don't care about physical interrupts routed over event
> channels can just test for the availability of the hypercall directly
> (HVMOP_set_evtchn_upcall_vector) without checking the CPUID bit.
> 
>>
>> Signed-off-by: Jane Malalane <jane.malalane@xxxxxxxxxx>
>> ---
>> CC: Jan Beulich <jbeulich@xxxxxxxx>
>> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> CC: "Roger Pau Monné" <roger.pau@xxxxxxxxxx>
>> CC: Wei Liu <wl@xxxxxxx>
>> ---
>>   xen/arch/x86/include/asm/domain.h   | 8 +++++++-
>>   xen/arch/x86/traps.c                | 3 +++
>>   xen/include/public/arch-x86/cpuid.h | 2 ++
>>   3 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/x86/include/asm/domain.h 
>> b/xen/arch/x86/include/asm/domain.h
>> index 35898d725f..f044e0a492 100644
>> --- 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
> 
> I think you should remove LAPIC here.  There's no such thing as 'LAPIC
> vectors', it's just that the old mechanism was bypassing the lapic
> EOI.
> 
>> + * 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))
>>   #define is_hvm_pv_evtchn_vcpu(v) (is_hvm_pv_evtchn_domain(v->domain))
>>   #define is_domain_direct_mapped(d) ((void)(d), 0)
>>   
>> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
>> index 25bffe47d7..2c51faab2c 100644
>> --- a/xen/arch/x86/traps.c
>> +++ b/xen/arch/x86/traps.c
>> @@ -1152,6 +1152,9 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, 
>> uint32_t leaf,
>>           res->a |= XEN_HVM_CPUID_DOMID_PRESENT;
>>           res->c = d->domain_id;
>>   
>> +        /* Per-vCPU event channel upcalls are implemented. */
> 
> ... are implemented and work correctly with PIRQs routed over event
> channels.
> 
>> +        res->a |= XEN_HVM_CPUID_UPCALL_VECTOR;
>> +
>>           break;
>>   
>>       case 5: /* PV-specific parameters */
>> diff --git a/xen/include/public/arch-x86/cpuid.h 
>> b/xen/include/public/arch-x86/cpuid.h
>> index f2b2b3632c..1760e2c405 100644
>> --- a/xen/include/public/arch-x86/cpuid.h
>> +++ b/xen/include/public/arch-x86/cpuid.h
>> @@ -109,6 +109,8 @@
>>    * field from 8 to 15 bits, allowing to target APIC IDs up 32768.
>>    */
>>   #define XEN_HVM_CPUID_EXT_DEST_ID      (1u << 5)
>> +/* Per-vCPU event channel upcalls. */
> 
> I would maybe expand the message to:
> 
> "Per-vCPU event channel upcalls work correctly with physical IRQs bound
> to event channels."

Thanks. Yes, if others agree that the CPUID bit can still take this name 
if better explained, despite it being, as you say, quite generic, I will 
add these comments in a v2.

Thanks,

Jane.

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.