Re: [PATCH 5/5] qemu: Add support for setting the TSEG size

[Ján Tomko <jtomko@xxxxxxxxxx>]

On Mon, May 21, 2018 at 05:00:53PM +0200, Martin Kletzander wrote:
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1469338
>
> Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
> ---
> src/qemu/qemu_command.c                       | 18 ++++
> src/qemu/qemu_domain.c                        | 84 +++++++++++++++++++
> .../qemuxml2argvdata/tseg-explicit-size.args  | 28 +++++++
> tests/qemuxml2argvdata/tseg-explicit-size.xml | 23 +++++
> tests/qemuxml2argvdata/tseg-i440fx.xml        | 23 +++++
> tests/qemuxml2argvdata/tseg-invalid-size.xml  | 23 +++++
> .../tseg-old-machine-type.args                | 27 ++++++
> .../tseg-old-machine-type.xml                 | 21 +++++
> tests/qemuxml2argvdata/tseg.args              | 28 +++++++
> tests/qemuxml2argvdata/tseg.xml               | 21 +++++
> tests/qemuxml2argvtest.c                      | 48 +++++++++++
> .../qemuxml2xmloutdata/tseg-explicit-size.xml | 46 ++++++++++
> .../tseg-old-machine-type.xml                 | 44 ++++++++++
> tests/qemuxml2xmloutdata/tseg.xml             | 46 ++++++++++
> tests/qemuxml2xmltest.c                       | 25 ++++++
> 15 files changed, 505 insertions(+)
> create mode 100644 tests/qemuxml2argvdata/tseg-explicit-size.args
> create mode 100644 tests/qemuxml2argvdata/tseg-explicit-size.xml
> create mode 100644 tests/qemuxml2argvdata/tseg-i440fx.xml
> create mode 100644 tests/qemuxml2argvdata/tseg-invalid-size.xml
> create mode 100644 tests/qemuxml2argvdata/tseg-old-machine-type.args
> create mode 100644 tests/qemuxml2argvdata/tseg-old-machine-type.xml
> create mode 100644 tests/qemuxml2argvdata/tseg.args
> create mode 100644 tests/qemuxml2argvdata/tseg.xml
> create mode 100644 tests/qemuxml2xmloutdata/tseg-explicit-size.xml
> create mode 100644 tests/qemuxml2xmloutdata/tseg-old-machine-type.xml
> create mode 100644 tests/qemuxml2xmloutdata/tseg.xml

> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 881d0ea46a75..3ea9e3d47344 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -3319,6 +3319,87 @@ qemuDomainDefCPUPostParse(virDomainDefPtr def)
> }
>
>
> +static int
> +qemuDomainSetDefaultTsegSize(virDomainDef *def,
> +                             virQEMUCapsPtr qemuCaps)
> +{
> +    const char *machine = NULL;
> +    char *end_ptr = NULL;
> +    unsigned int major = 0;
> +    unsigned int minor = 0;
> +
> +    def->tseg_size = 0;
> +
> +    if (!qemuDomainIsQ35(def))
> +        return 0;
> +
> +    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MCH_EXTENDED_TSEG_MBYTES))
> +        return 0;
> +
> +    machine = STRSKIP(def->os.machine, "pc-q35-");
> +
> +    if (!machine)
> +        goto error;
> +
> +    if (virStrToLong_uip(machine, &end_ptr, 10, &major) < 0)
> +        goto error;
> +
> +    if (*end_ptr != '.')
> +        goto error;
> +
> +    machine = end_ptr + 1;
> +
> +    if (virStrToLong_uip(machine, &end_ptr, 10, &minor) < 0)
> +        goto error;
> +    if (*end_ptr != '\0')
> +        goto error;
> +
> +    /* QEMU started defaulting to 16MiB after 2.9 */
> +    if (major > 2 || (major == 2 && minor > 9))

Please use virParseVersionString

> +        def->tseg_size = 16 * 1024 * 1024;
> +
> +    return 0;
> +
> + error:
> +    virReportError(VIR_ERR_INTERNAL_ERROR,
> +                   _("Cannot parse QEMU machine type version '%s'"),
> +                   def->os.machine);
> +    return -1;
> +}
> +
> +
> +static int
> +qemuDomainDefTsegPostParse(virDomainDefPtr def,
> +                           virQEMUCapsPtr qemuCaps)
> +{
> +    if (def->features[VIR_DOMAIN_FEATURE_SMM] != VIR_TRISTATE_SWITCH_ON)
> +        return 0;
> +
> +    if (!def->tseg_size)
> +        return qemuDomainSetDefaultTsegSize(def, qemuCaps);
> +
> +    if (!qemuDomainIsQ35(def)) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("SMM TSEG is only supported with q35 machine type"));
> +        return -1;
> +    }
> +
> +    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MCH_EXTENDED_TSEG_MBYTES)) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("Setting TSEG size is not supported with this QEMU binary"));
> +        return -1;
> +    }
> +
> +    if (VIR_ROUND_UP(def->tseg_size, 1024 * 1024) != def->tseg_size) {

Interesting way of writing the % operator.

> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("SMM TSEG size must be divisible by 1 MiB"));
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +
> static int
> qemuDomainDefPostParseBasic(virDomainDefPtr def,
>                             virCapsPtr caps,

I'm not sure whether this trial-and-error attribute angers some purists,
but they'll have plenty of time to object until the freeze is over.

Reviewed-by: Ján Tomko <jtomko@xxxxxxxxxx>

Jano

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list