[v1] xen/domain: updates to hardware emulation flags

[PATCH v1 2/3] x86/domain: add helpers to simplify emulation flags management

Posted by dmkhn@proton.me 10 months, 2 weeks ago

From: Denis Mukhin <dmukhin@ford.com>

Introduce XEN_X86_EMU_BASELINE and XEN_X86_EMU_OPTIONAL to simplify
d->arch.emulation_flags management in the code.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 tools/python/xen/lowlevel/xc/xc.c | 4 +---
 xen/include/public/arch-x86/xen.h | 7 +++++++
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
index 9feb12ae2b..2c01d47575 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -159,9 +159,7 @@ static PyObject *pyxc_domain_create(XcObject *self,
 
 #if defined (__i386) || defined(__x86_64__)
     if ( config.flags & XEN_DOMCTL_CDF_hvm )
-        config.arch.emulation_flags = XEN_X86_EMU_ALL &
-                                      ~(XEN_X86_EMU_VPCI |
-                                        XEN_X86_EMU_USE_PIRQ);
+        config.arch.emulation_flags = XEN_X86_EMU_BASELINE;
 #elif defined (__arm__) || defined(__aarch64__)
     config.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
 #else
diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
index fc24879866..55802720f8 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -290,6 +290,13 @@ struct xen_arch_domainconfig {
                                      XEN_X86_EMU_VGA | XEN_X86_EMU_IOMMU |   \
                                      XEN_X86_EMU_PIT | XEN_X86_EMU_USE_PIRQ |\
                                      XEN_X86_EMU_VPCI)
+
+#define XEN_X86_EMU_OPTIONAL        (XEN_X86_EMU_VPCI | \
+                                     XEN_X86_EMU_USE_PIRQ)
+
+#define XEN_X86_EMU_BASELINE        (XEN_X86_EMU_ALL & ~XEN_X86_EMU_OPTIONAL)
+
+    /* Hardware emulation flags. */
     uint32_t emulation_flags;
 
 /*
-- 
2.34.1

Re: [PATCH v1 2/3] x86/domain: add helpers to simplify emulation flags management

Posted by Jan Beulich 10 months, 1 week ago

On 01.04.2025 02:52, dmkhn@proton.me wrote:
> From: Denis Mukhin <dmukhin@ford.com>
> 
> Introduce XEN_X86_EMU_BASELINE and XEN_X86_EMU_OPTIONAL to simplify
> d->arch.emulation_flags management in the code.

If the simplification is limited to ...

> --- a/tools/python/xen/lowlevel/xc/xc.c
> +++ b/tools/python/xen/lowlevel/xc/xc.c
> @@ -159,9 +159,7 @@ static PyObject *pyxc_domain_create(XcObject *self,
>  
>  #if defined (__i386) || defined(__x86_64__)
>      if ( config.flags & XEN_DOMCTL_CDF_hvm )
> -        config.arch.emulation_flags = XEN_X86_EMU_ALL &
> -                                      ~(XEN_X86_EMU_VPCI |
> -                                        XEN_X86_EMU_USE_PIRQ);
> +        config.arch.emulation_flags = XEN_X86_EMU_BASELINE;
>  #elif defined (__arm__) || defined(__aarch64__)
>      config.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
>  #else

... just this, I'm not convinced that's worth introducing yet two more items
into the namespace.

> --- a/xen/include/public/arch-x86/xen.h
> +++ b/xen/include/public/arch-x86/xen.h
> @@ -290,6 +290,13 @@ struct xen_arch_domainconfig {
>                                       XEN_X86_EMU_VGA | XEN_X86_EMU_IOMMU |   \
>                                       XEN_X86_EMU_PIT | XEN_X86_EMU_USE_PIRQ |\
>                                       XEN_X86_EMU_VPCI)
> +
> +#define XEN_X86_EMU_OPTIONAL        (XEN_X86_EMU_VPCI | \
> +                                     XEN_X86_EMU_USE_PIRQ)
> +
> +#define XEN_X86_EMU_BASELINE        (XEN_X86_EMU_ALL & ~XEN_X86_EMU_OPTIONAL)
> +
> +    /* Hardware emulation flags. */
>      uint32_t emulation_flags;

The comment isn't quite accurate here (and hence perhaps also not in the
earlier patch): XEN_X86_EMU_USE_PIRQ isn't exactly about emulation of anything
hardware-ish.

Jan

Re: [PATCH v1 2/3] x86/domain: add helpers to simplify emulation flags management

Posted by dmkhn@proton.me 9 months, 3 weeks ago

On Tue, Apr 08, 2025 at 05:23:17PM +0200, Jan Beulich wrote:
> On 01.04.2025 02:52, dmkhn@proton.me wrote:
> > From: Denis Mukhin <dmukhin@ford.com>
> >
> > Introduce XEN_X86_EMU_BASELINE and XEN_X86_EMU_OPTIONAL to simplify
> > d->arch.emulation_flags management in the code.
> 
> If the simplification is limited to ...
> 
> > --- a/tools/python/xen/lowlevel/xc/xc.c
> > +++ b/tools/python/xen/lowlevel/xc/xc.c
> > @@ -159,9 +159,7 @@ static PyObject *pyxc_domain_create(XcObject *self,
> >
> >  #if defined (__i386) || defined(__x86_64__)
> >      if ( config.flags & XEN_DOMCTL_CDF_hvm )
> > -        config.arch.emulation_flags = XEN_X86_EMU_ALL &
> > -                                      ~(XEN_X86_EMU_VPCI |
> > -                                        XEN_X86_EMU_USE_PIRQ);
> > +        config.arch.emulation_flags = XEN_X86_EMU_BASELINE;
> >  #elif defined (__arm__) || defined(__aarch64__)
> >      config.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
> >  #else
> 
> ... just this, I'm not convinced that's worth introducing yet two more items
> into the namespace.

Sorry, it took me a while to get back to the series.

The X86_EMU_BASELINE mask is used in two places: in the toolstack (xc.c) and
in domain.c, emulation_flags_ok(), so adding a new flag may require updates
in two places.

Perhaps it will be better to merge this patch 2 with patch 3 to highlight that? 

> 
> > --- a/xen/include/public/arch-x86/xen.h
> > +++ b/xen/include/public/arch-x86/xen.h
> > @@ -290,6 +290,13 @@ struct xen_arch_domainconfig {
> >                                       XEN_X86_EMU_VGA | XEN_X86_EMU_IOMMU |   \
> >                                       XEN_X86_EMU_PIT | XEN_X86_EMU_USE_PIRQ |\
> >                                       XEN_X86_EMU_VPCI)
> > +
> > +#define XEN_X86_EMU_OPTIONAL        (XEN_X86_EMU_VPCI | \
> > +                                     XEN_X86_EMU_USE_PIRQ)
> > +
> > +#define XEN_X86_EMU_BASELINE        (XEN_X86_EMU_ALL & ~XEN_X86_EMU_OPTIONAL)
> > +
> > +    /* Hardware emulation flags. */
> >      uint32_t emulation_flags;
> 
> The comment isn't quite accurate here (and hence perhaps also not in the
> earlier patch): XEN_X86_EMU_USE_PIRQ isn't exactly about emulation of anything
> hardware-ish.
> 
> Jan

Re: [PATCH v1 2/3] x86/domain: add helpers to simplify emulation flags management

Posted by Jan Beulich 9 months, 3 weeks ago

On 23.04.2025 00:49, dmkhn@proton.me wrote:
> On Tue, Apr 08, 2025 at 05:23:17PM +0200, Jan Beulich wrote:
>> On 01.04.2025 02:52, dmkhn@proton.me wrote:
>>> From: Denis Mukhin <dmukhin@ford.com>
>>>
>>> Introduce XEN_X86_EMU_BASELINE and XEN_X86_EMU_OPTIONAL to simplify
>>> d->arch.emulation_flags management in the code.
>>
>> If the simplification is limited to ...
>>
>>> --- a/tools/python/xen/lowlevel/xc/xc.c
>>> +++ b/tools/python/xen/lowlevel/xc/xc.c
>>> @@ -159,9 +159,7 @@ static PyObject *pyxc_domain_create(XcObject *self,
>>>
>>>  #if defined (__i386) || defined(__x86_64__)
>>>      if ( config.flags & XEN_DOMCTL_CDF_hvm )
>>> -        config.arch.emulation_flags = XEN_X86_EMU_ALL &
>>> -                                      ~(XEN_X86_EMU_VPCI |
>>> -                                        XEN_X86_EMU_USE_PIRQ);
>>> +        config.arch.emulation_flags = XEN_X86_EMU_BASELINE;
>>>  #elif defined (__arm__) || defined(__aarch64__)
>>>      config.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
>>>  #else
>>
>> ... just this, I'm not convinced that's worth introducing yet two more items
>> into the namespace.
> 
> Sorry, it took me a while to get back to the series.
> 
> The X86_EMU_BASELINE mask is used in two places: in the toolstack (xc.c) and
> in domain.c, emulation_flags_ok(), so adding a new flag may require updates
> in two places.

Which may be even desirable in this case, so effects can be properly considered
at both sites. I wouldn't take it for given that the two will forever need to
stay in full sync.

Jan