[PATCH 1/1] set vm physical bits(phys_bits)

[Tim Wiederhake]

On Thu, 2021-03-25 at 14:45 +0800, Paul Schlacter wrote:
> > Set the vm phys_bits through the phys and hostphysbits in XML
> > <phys bits='43' /> corresponds to "-cpu-phys-bits=42"
Is the 43 -> 42 change a typo? I do not see the "-1" in the code below.

> > <hostphysbits /> corresponds to "host-phys-bits=on"
> > 
> > <cpu mode='host-passthrough' check='none'>
> >     <phys bits='43' />
> >     <hostphysbits />

This looks odd to me, I would have expected something like "<phys
bits='43' hostphysbits='on'/>" or "<hostphysbits state='on'/>".
Additionally, if this is qemu-specific and not applicable to other
hypervisors, it might be better if this goes into the features/kvm
section. Note that I am not in a position to decide on that and you
need feedback from other libvirt maintainers.

> > </cpu>
> > ---
> >  docs/schemas/cputypes.rng | 20 ++++++++++++++++++++
> >  src/conf/cpu_conf.c       | 34 ++++++++++++++++++++++++++++++++++
> >  src/conf/cpu_conf.h       |  2 ++
> >  src/qemu/qemu_command.c   |  6 ++++++
> >  4 files changed, 62 insertions(+)
> > 
> > diff --git a/docs/schemas/cputypes.rng b/docs/schemas/cputypes.rng
> > index 77c8fa783b..fb8a14ddea 100644
> > --- a/docs/schemas/cputypes.rng
> > +++ b/docs/schemas/cputypes.rng
> > @@ -300,6 +300,20 @@
> >      </element>
> >    </define>
> >  
> > +  <define name="cpuPhysBits">
> > +    <element name="phys">
> > +      <attribute name="bits">
> > +        <ref name="positiveInteger"/>
> > +      </attribute>
> > +    </element>
> > +  </define>
> > +
> > +  <define name="cpuHostPhysBits">
> > +    <element name="hostphysbits">
> > +      <empty/>
> > +    </element>
> > +  </define>
> > +
> >    <define name="hostcpu">
> >      <element name="cpu">
> >        <element name="arch">
> > @@ -414,6 +428,12 @@
> >          <optional>
> >            <ref name="cpuCache"/>
> >          </optional>
> > +        <optional>
> > +          <ref name="cpuPhysBits"/>
> > +        </optional>
> > +        <optional>
> > +          <ref name="cpuHostPhysBits"/>
> > +        </optional>
> >        </interleave>
> >      </element>
> >    </define>
> > diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
> > index 380a74691d..18d788c528 100644
> > --- a/src/conf/cpu_conf.c
> > +++ b/src/conf/cpu_conf.c
> > @@ -158,6 +158,8 @@ virCPUDefCopyModelFilter(virCPUDefPtr dst,
> >      dst->model = g_strdup(src->model);
> >      dst->vendor = g_strdup(src->vendor);
> >      dst->vendor_id = g_strdup(src->vendor_id);
> > +    dst->phys_bits = src->phys_bits;
> > +    dst->host_phys_bits = src->host_phys_bits;
> >      dst->microcodeVersion = src->microcodeVersion;
> >      dst->nfeatures_max = src->nfeatures;
> >      dst->nfeatures = 0;
> > @@ -540,6 +542,18 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
> >          return -1;
> >      }
> >  
> > +    if (virXPathNode("./phys[1]", ctxt)) {
> > +        unsigned int phys_bits;
> > +        if (virXPathUInt("string(./phys[1]/@bits)",
> > +                ctxt, &phys_bits) >=0 ) {
Indentation looks off

> > +            def->phys_bits = (unsigned int) phys_bits;
> > +        }
By using virXPathUint I believe you can directly pass def->phys_bits in
and do not need the local variable any longer.

> > +    }
> > +
> > +    if (virXPathNode("./hostphysbits[1]", ctxt)) {
> > +        def->host_phys_bits = true;
> > +    }
> > +
> >      if (virXPathNode("./topology[1]", ctxt)) {
> >          unsigned long ul;
> >  
> > @@ -811,6 +825,12 @@ virCPUDefFormatBuf(virBufferPtr buf,
> >          virBufferAddLit(buf, "/>\n");
> >      }
> >  
> > +    if (def->phys_bits > 0)
> > +        virBufferAsprintf(buf, "<phys bits='%u' />\n", def-
> > >phys_bits);
> > +
> > +    if (def->host_phys_bits)
> > +        virBufferAddLit(buf, "<hostphysbits />\n");
> > +
> >      if (def->sockets && def->dies && def->cores && def->threads) {
> >          virBufferAddLit(buf, "<topology");
> >          virBufferAsprintf(buf, " sockets='%u'", def->sockets);
> > @@ -1067,6 +1087,20 @@ virCPUDefIsEqual(virCPUDefPtr src,
> >          return false;
> >      }
> >  
> > +    if (src->phys_bits != dst->phys_bits) {
> > +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > +                       _("Target CPU phys_bits %d does not match
> > source %d"),
> > +                       dst->phys_bits, src->phys_bits);
> > +        goto cleanup;
> > +    }
> > +
> > +    if (src->host_phys_bits != dst->host_phys_bits) {
> > +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > +                       _("Target CPU host_phys_bits %d does not
> > match source %d"),
> > +                       dst->host_phys_bits, src->host_phys_bits);
> > +        goto cleanup;
> > +    }
> > +
> >      if (src->sockets != dst->sockets) {
> >          MISMATCH(_("Target CPU sockets %d does not match source
> > %d"),
> >                   dst->sockets, src->sockets);
> > diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
> > index 7ab198d370..f2a23ad41e 100644
> > --- a/src/conf/cpu_conf.h
> > +++ b/src/conf/cpu_conf.h
> > @@ -132,6 +132,8 @@ struct _virCPUDef {
> >      char *vendor_id;    /* vendor id returned by CPUID in the
> > guest */
> >      int fallback;       /* enum virCPUFallback */
> >      char *vendor;
> > +    unsigned int phys_bits;
> > +    bool host_phys_bits;
> >      unsigned int microcodeVersion;
> >      unsigned int sockets;
> >      unsigned int dies;
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index 1b4fa77867..d9bf3d5ce8 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -6729,6 +6729,12 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
> >              virBufferAddLit(&buf, ",l3-cache=off");
> >      }
> >  
> > +    if (def->cpu && def->cpu->phys_bits > 0)
> > +        virBufferAsprintf(&buf, ",phys-bits=%u", def->cpu-
> > >phys_bits);
> > +
> > +    if (def->cpu && def->cpu->host_phys_bits)
> > +        virBufferAddLit(&buf, ",host-phys-bits=on");
> > +
> >      cpu = virBufferContentAndReset(&cpu_buf);
> >      cpu_flags = virBufferContentAndReset(&buf);
> >  
> > -- 
> > 2.24.3 (Apple Git-128)