[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [QEMU][PATCH v2 10/11] hw/arm: introduce xenpv machine
From: |
Alex Bennée |
Subject: |
Re: [QEMU][PATCH v2 10/11] hw/arm: introduce xenpv machine |
Date: |
Fri, 02 Dec 2022 14:45:58 +0000 |
User-agent: |
mu4e 1.9.3; emacs 29.0.60 |
Vikram Garhwal <vikram.garhwal@amd.com> writes:
> Add a new machine xenpv which creates a IOREQ server to register/connect with
> Xen Hypervisor.
>
> Optional: When CONFIG_TPM is enabled, it also creates a tpm-tis-device, adds a
> TPM emulator and connects to swtpm running on host machine via chardev socket
> and support TPM functionalities for a guest domain.
>
> Extra command line for aarch64 xenpv QEMU to connect to swtpm:
> -chardev socket,id=chrtpm,path=/tmp/myvtpm2/swtpm-sock \
> -tpmdev emulator,id=tpm0,chardev=chrtpm \
>
> swtpm implements a TPM software emulator(TPM 1.2 & TPM 2) built on libtpms and
> provides access to TPM functionality over socket, chardev and CUSE interface.
> Github repo: https://github.com/stefanberger/swtpm
> Example for starting swtpm on host machine:
> mkdir /tmp/vtpm2
> swtpm socket --tpmstate dir=/tmp/vtpm2 \
> --ctrl type=unixio,path=/tmp/vtpm2/swtpm-sock &
>
> /* Comment about machine name. Will be removed in next version*/
> Please reply with the machine name you agree. Below are two possible names:
> 1. xenpv
> 2. xenpvh
>
> Signed-off-by: Vikram Garhwal <vikram.garhwal@amd.com>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> ---
> hw/arm/meson.build | 2 +
> hw/arm/xen_arm.c | 175 ++++++++++++++++++++++++++++++++++
> include/hw/arm/xen_arch_hvm.h | 9 ++
> include/hw/xen/arch_hvm.h | 2 +
> 4 files changed, 188 insertions(+)
> create mode 100644 hw/arm/xen_arm.c
> create mode 100644 include/hw/arm/xen_arch_hvm.h
>
> diff --git a/hw/arm/meson.build b/hw/arm/meson.build
> index 92f9f6e000..0cae024374 100644
> --- a/hw/arm/meson.build
> +++ b/hw/arm/meson.build
> @@ -62,5 +62,7 @@ arm_ss.add(when: 'CONFIG_FSL_IMX7', if_true:
> files('fsl-imx7.c', 'mcimx7d-sabre.
> arm_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmu-common.c',
> 'smmuv3.c'))
> arm_ss.add(when: 'CONFIG_FSL_IMX6UL', if_true: files('fsl-imx6ul.c',
> 'mcimx6ul-evk.c'))
> arm_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_soc.c'))
> +arm_ss.add(when: 'CONFIG_XEN', if_true: files('xen_arm.c'))
> +arm_ss.add_all(xen_ss)
>
> hw_arch += {'arm': arm_ss}
> diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
> new file mode 100644
> index 0000000000..bcb8e95f2c
> --- /dev/null
> +++ b/hw/arm/xen_arm.c
> @@ -0,0 +1,175 @@
> +/*
> + * QEMU ARM Xen PV Machine
> + *
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> copy
> + * of this software and associated documentation files (the "Software"), to
> deal
> + * in the Software without restriction, including without limitation the
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/error-report.h"
> +#include "qapi/qapi-commands-migration.h"
> +#include "hw/boards.h"
> +#include "hw/sysbus.h"
> +#include "sysemu/block-backend.h"
> +#include "sysemu/tpm_backend.h"
> +#include "sysemu/sysemu.h"
> +#include "hw/xen/xen-legacy-backend.h"
> +#include "hw/xen/xen-hvm-common.h"
> +#include "sysemu/tpm.h"
> +#include "hw/xen/arch_hvm.h"
> +
> +#define TYPE_XEN_ARM MACHINE_TYPE_NAME("xenpv")
> +OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM)
> +
> +static MemoryListener xen_memory_listener = {
> + .region_add = xen_region_add,
> + .region_del = xen_region_del,
> + .log_start = NULL,
> + .log_stop = NULL,
> + .log_sync = NULL,
> + .log_global_start = NULL,
> + .log_global_stop = NULL,
> + .priority = 10,
> +};
> +
> +struct XenArmState {
> + /*< private >*/
> + MachineState parent;
> +
> + XenIOState *state;
> +};
> +
> +void arch_handle_ioreq(XenIOState *state, ioreq_t *req)
> +{
> + hw_error("Invalid ioreq type 0x%x\n", req->type);
> +
> + return;
> +}
> +
> +void arch_xen_set_memory(XenIOState *state, MemoryRegionSection *section,
> + bool add)
> +{
> +}
> +
> +void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
> +{
> +}
> +
> +void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
> +{
> +}
> +
This function is only used under CONFIG_TPM so without it you get:
../../hw/arm/xen_arm.c:78:12: error: ‘xen_init_ioreq’ defined but not used
[-Werror=unused-function]
78 | static int xen_init_ioreq(XenIOState *state, unsigned int max_cpus)
| ^~~~~~~~~~~~~~
cc1: all warnings being treated as errors
I think I reported this on v1 as well.
> +static int xen_init_ioreq(XenIOState *state, unsigned int max_cpus)
> +{
> + xen_dmod = xendevicemodel_open(0, 0);
> + if (xen_dmod == NULL) {
> + perror("xen: can't open xen device model interface\n");
> + return -1;
> + }
> +
> + xen_xc = xc_interface_open(0, 0, 0);
> + if (xen_xc == NULL) {
> + perror("xen: can't open xen interface\n");
> + return -1;
> + }
> +
> + xen_fmem = xenforeignmemory_open(0, 0);
> + if (xen_fmem == NULL) {
> + perror("xen: can't open xen fmem interface\n");
> + xc_interface_close(xen_xc);
> + return -1;
> + }
> +
> + xen_register_ioreq(state, max_cpus, xen_memory_listener);
> +
> + xen_register_backend(state);
> +
> + xenstore_record_dm_state(state->xenstore, "running");
> +
> + return 0;
> +}
> +
> +static void xen_enable_tpm(void)
> +{
> +/* qemu_find_tpm_be is only available when CONFIG_TPM is enabled. */
> +#ifdef CONFIG_TPM
> + Error *errp = NULL;
> + DeviceState *dev;
> + SysBusDevice *busdev;
> +
> + TPMBackend *be = qemu_find_tpm_be("tpm0");
> + if (be == NULL) {
> + DPRINTF("Couldn't fine the backend for tpm0\n");
> + return;
> + }
> + dev = qdev_new(TYPE_TPM_TIS_SYSBUS);
> + object_property_set_link(OBJECT(dev), "tpmdev", OBJECT(be), &errp);
> + object_property_set_str(OBJECT(dev), "tpmdev", be->id, &errp);
> + busdev = SYS_BUS_DEVICE(dev);
> + sysbus_realize_and_unref(busdev, &error_fatal);
> + sysbus_mmio_map(busdev, 0, GUEST_TPM_BASE);
> +
> + DPRINTF("Connected tpmdev at address 0x%lx\n", GUEST_TPM_BASE);
> +#endif
> +}
> +
> +static void xen_arm_init(MachineState *machine)
> +{
> + XenArmState *xam = XEN_ARM(machine);
> +
> + xam->state = g_new0(XenIOState, 1);
> +
> + /* For now enable IOREQ only for CONFIG_TPM. */
> +#ifdef CONFIG_TPM
> + if (xen_init_ioreq(xam->state, machine->smp.cpus)) {
> + return;
> + }
> +#endif
> +
> + xen_enable_tpm();
In fact this just looks weird - you guard the call to init_ioreq with
ifdefs guarding it but nest those ifdefs in xen_enable_tpm. Surely you
want the IOREQ server support even if you don't have TPM. Otherwise it
should be a hard configure requirement.
> +
> + return;
> +}
> +
> +static void xen_arm_machine_class_init(ObjectClass *oc, void *data)
> +{
> +
> + MachineClass *mc = MACHINE_CLASS(oc);
> + mc->desc = "Xen Para-virtualized PC";
> + mc->init = xen_arm_init;
> + mc->max_cpus = 1;
> +
> +#ifdef CONFIG_TPM
> + machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
> +#endif
> +}
> +
> +static const TypeInfo xen_arm_machine_type = {
> + .name = TYPE_XEN_ARM,
> + .parent = TYPE_MACHINE,
> + .class_init = xen_arm_machine_class_init,
> + .instance_size = sizeof(XenArmState),
> +};
> +
> +static void xen_arm_machine_register_types(void)
> +{
> + type_register_static(&xen_arm_machine_type);
> +}
> +
> +type_init(xen_arm_machine_register_types)
> diff --git a/include/hw/arm/xen_arch_hvm.h b/include/hw/arm/xen_arch_hvm.h
> new file mode 100644
> index 0000000000..8fd645e723
> --- /dev/null
> +++ b/include/hw/arm/xen_arch_hvm.h
> @@ -0,0 +1,9 @@
> +#ifndef HW_XEN_ARCH_ARM_HVM_H
> +#define HW_XEN_ARCH_ARM_HVM_H
> +
> +#include <xen/hvm/ioreq.h>
> +void arch_handle_ioreq(XenIOState *state, ioreq_t *req);
> +void arch_xen_set_memory(XenIOState *state,
> + MemoryRegionSection *section,
> + bool add);
> +#endif
> diff --git a/include/hw/xen/arch_hvm.h b/include/hw/xen/arch_hvm.h
> index 26674648d8..c7c515220d 100644
> --- a/include/hw/xen/arch_hvm.h
> +++ b/include/hw/xen/arch_hvm.h
> @@ -1,3 +1,5 @@
> #if defined(TARGET_I386) || defined(TARGET_X86_64)
> #include "hw/i386/xen_arch_hvm.h"
> +#elif defined(TARGET_ARM) || defined(TARGET_ARM_64)
> +#include "hw/arm/xen_arch_hvm.h"
> #endif
--
Alex Bennée