[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH v3 09/28] xen/vm_event: consolidate CONFIG_VM_EVENT
- To: Jan Beulich <jbeulich@xxxxxxxx>, Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
- From: "Penny, Zheng" <penny.zheng@xxxxxxx>
- Date: Tue, 11 Nov 2025 07:08:01 +0000
- Accept-language: en-US
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; 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=pP7lJJsLPhy1oqOsULEvMvu97coRCOdXjbh/x/rTAs8=; b=YQpBzme818Qdt6oEmtvAR+mEZzaWQv5FGVipOtOAT05y+pYnJWEZPoAV2FLE9WrTagDwzItCnPZBa6PbURsCr9ScJywMN/yhYdRIi4uYiW7GuHHfLiz6nkwXxin1e5miPeP/ecIdd+LQeR0ea8EqRq1tM1f1oEUiHJnIRSZaE7RI1fl+ETepc9AsQoeDpWRMpeZj+kgVEVeNYb5ND49xEjQefedlTxlMdjVN+NMKm8cAjSHMeXR0dAEm16KhIECV41tPDYlUxgFkbX2uoLRwFliXW1AZCMM/zBxG+E9BSeISviovqQy12QMESk6RJ+OWwFGYjEnUjmKVDCP3qqtWeQ==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=Id111xkmCRbKxIQi679UzPymjOia5W1Dx51eVgsDmodC12EaNkD4aViBQr5fz0da2c1GN9dYBaUVM9fM8/t40WOout4MXmh25wHLtSoDlu3AmXXuhDCJEBAL5XjPVN2XPVOkOvi+HP7KqMP7CeXtdiPSu6rNAZRUKwzWB7GZbzxonNxfj73ARxnxJP/LF+T1kjVyLCT+OGwRsPbhvUD/xhSRnePu4bVSJOluWXsgtifkFeFf3szN/mwMaIARqw/mm6HhZVUqRQpF7Yu0AsAzUk3DpDloVM+U5yn584Rmdx3qejXJsNzOEqz8S7OEtK8LkZRL3+48EtlivueF8Z6I7A==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com;
- Cc: "Huang, Ray" <Ray.Huang@xxxxxxx>, "oleksii.kurochko@xxxxxxxxx" <oleksii.kurochko@xxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxxx>, "Orzel, Michal" <Michal.Orzel@xxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx>, Petre Pircalabu <ppircalabu@xxxxxxxxxxxxxxx>, "Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- Delivery-date: Tue, 11 Nov 2025 07:08:23 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
- Msip_labels: MSIP_Label_f265efc6-e181-49d6-80f4-fae95cf838a0_Enabled=True;MSIP_Label_f265efc6-e181-49d6-80f4-fae95cf838a0_SiteId=3dd8961f-e488-4e60-8e11-a82d994e183d;MSIP_Label_f265efc6-e181-49d6-80f4-fae95cf838a0_SetDate=2025-11-11T07:07:08.0000000Z;MSIP_Label_f265efc6-e181-49d6-80f4-fae95cf838a0_Name=Open Source;MSIP_Label_f265efc6-e181-49d6-80f4-fae95cf838a0_ContentBits=3;MSIP_Label_f265efc6-e181-49d6-80f4-fae95cf838a0_Method=Privileged
- Thread-index: AQHcPCqA+mY6s9fCEkmLa9SlVlA0PrTZYSAAgBPVwiA=
- Thread-topic: [PATCH v3 09/28] xen/vm_event: consolidate CONFIG_VM_EVENT
[Public]
Hi,
Sorry for the late response. Just got back from long annual leaves
> -----Original Message-----
> > --- a/xen/arch/x86/include/asm/mem_access.h
> > +++ b/xen/arch/x86/include/asm/mem_access.h
> > @@ -14,6 +14,7 @@
> > #ifndef __ASM_X86_MEM_ACCESS_H__
> > #define __ASM_X86_MEM_ACCESS_H__
> >
> > +#ifdef CONFIG_VM_EVENT
> > /*
> > * Setup vm_event request based on the access (gla is -1ull if not
> > available).
> > * Handles the rw2rx conversion. Boolean return value indicates if event
> > type
> > @@ -25,6 +26,14 @@
> > bool p2m_mem_access_check(paddr_t gpa, unsigned long gla,
> > struct npfec npfec,
> > struct vm_event_st **req_ptr);
> > +#else
> > +static inline bool p2m_mem_access_check(paddr_t gpa, unsigned long gla,
> > + struct npfec npfec,
> > + struct vm_event_st **req_ptr)
> > +{
> > + return false;
>
> Leaving *req_ptr untouched feels dangerous; the fact that the sole caller has
> what it uses set to NULL up front is secondary.
>
If we *req_ptr = NULL; compiler will not DCE the following code block when
VM_EVENT=n:
```
if ( req_ptr )
{
if ( monitor_traps(curr, sync, req_ptr) < 0 )
rc = 0;
xfree(req_ptr);
}
return rc;
```
Or am I misunderstanding what you suggest?
> From looking at the function it's also not quite clear to me whether "false"
> is
> the correct return value here. Tamas?
>
> > --- a/xen/arch/x86/include/asm/monitor.h
> > +++ b/xen/arch/x86/include/asm/monitor.h
> > @@ -32,6 +32,7 @@ struct monitor_msr_bitmap {
> > DECLARE_BITMAP(high, 8192);
> > };
> >
> > +#ifdef COMFIG_VM_EVENT
>
> Typo aside, isn't the entire file (perhaps minus some stubs) useful only when
> VM_EVENT=y?
>
Yes, maybe only arch_monitor_get_capabilities() needs the wrapping. As it
invokes hvm_has_set_descriptor_access_exiting(), which is declared only when
VM_EVENT=y
> Jan
|
