diff mbox series

[v1,10/12] hw/arm: introduce xenpv machine

Message ID 20221015050750.4185-11-vikram.garhwal@amd.com (mailing list archive)
State New, archived
Headers show
Series [v1,01/12] hw/xen: Correct build config for xen_pt_stub | expand

Commit Message

Vikram Garhwal Oct. 15, 2022, 5:07 a.m. UTC
Add a new machine xenpv which creates a IOREQ server to register/connect with
Xen Hypervisor.

Xen IOREQ connection expect the TARGET_PAGE_SIZE to 4096, and the xenpv machine
on ARM will have no CPU definitions. We need to define TARGET_PAGE_SIZE
appropriately ourselves.

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 &

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              | 163 ++++++++++++++++++++++++++++++++++
 include/hw/arm/xen_arch_hvm.h |  12 +++
 include/hw/xen/arch_hvm.h     |   2 +
 4 files changed, 179 insertions(+)
 create mode 100644 hw/arm/xen_arm.c
 create mode 100644 include/hw/arm/xen_arch_hvm.h

Comments

Julien Grall Oct. 16, 2022, 5:47 p.m. UTC | #1
Hi,

There seem to be some missing patches on xen-devel (including the cover 
letter). Is that expected?

On 15/10/2022 06:07, Vikram Garhwal wrote:
> Add a new machine xenpv which creates a IOREQ server to register/connect with
> Xen Hypervisor.

I don't like the name 'xenpv' because it doesn't convey the fact that 
some of the HW may be emulated rather than para-virtualized. In fact one 
may only want to use for emulating devices.

Potential name would be 'xen-arm' or re-using 'virt' but with 
'accel=xen' to select a Xen layout.

> 
> Xen IOREQ connection expect the TARGET_PAGE_SIZE to 4096, and the xenpv machine
> on ARM will have no CPU definitions. We need to define TARGET_PAGE_SIZE
> appropriately ourselves.
> 
> 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 &

I see patches for QEMU but not Xen. How can this be tested with existing 
Xen? Will libxl ever create QEMU?

[...]

> +static int xen_init_ioreq(XenIOState *state, unsigned int max_cpus)
> +{
> +    xen_dmod = xendevicemodel_open(0, 0);
> +    xen_xc = xc_interface_open(0, 0, 0);
> +
> +    if (xen_xc == NULL) {

You are checking xen_xc but not xen_dmod. Why?

> +        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);
> +
> +    xenstore_record_dm_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);

I can't find where GUEST_TPM_BASE is defined. But then the guest memory 
layout is not expected to be stable. With your current approach, it 
means QEMU would need to be rebuilt for every Xen version. Is it what we 
want?

> +
> +    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);
> +
> +    if (xen_init_ioreq(xam->state, machine->smp.cpus)) {
> +        return;

In another patch, you said the IOREQ would be optional. IHMO, I think 
this is a bad idea to register it by default because one may only want 
to use PV drivers. Registering IOREQ will add unnecessary overhead in Xen.

Furthermore, it means that someone selecting TPM but Xen is not built 
with CONFIG_IOREQ=y (BTW This is still a tech preview but there are 
security holes on Arm...) will not get an error. Instead, the OS will 
until it crashes when trying to access the TPM.

Overall I think it would be better if IOREQ is only registered when a 
device requires (like TPM) it *and* throw an error if there is a problem 
during the initialization.

> +    } > +
> +    xen_enable_tpm();
> +
> +    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;
> +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);

Shouldn't this be protected with #ifdef CONFIG_TPM?

> +}
> +
> +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..f645dfec28
> --- /dev/null
> +++ b/include/hw/arm/xen_arch_hvm.h
> @@ -0,0 +1,12 @@
> +#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);
> +
> +#undef TARGET_PAGE_SIZE

I am a bit puzzled with this #undef. In the commit message you said that 
there will be no CPU definition. So the implications is this should not 
be defined.

If it is defined, then what guarantees that all the source will use the 
correct value?


> +#define TARGET_PAGE_SIZE 4096

It would be better to use XC_PAGE_SIZE (or similar) rather than 
hardcoding it.

> +#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

Cheers,
Stefano Stabellini Oct. 18, 2022, 1:26 a.m. UTC | #2
On Sun, 16 Oct 2022, Julien Grall wrote:
> Hi,
> 
> There seem to be some missing patches on xen-devel (including the cover
> letter). Is that expected?
> 
> On 15/10/2022 06:07, Vikram Garhwal wrote:
> > Add a new machine xenpv which creates a IOREQ server to register/connect
> > with
> > Xen Hypervisor.
> 
> I don't like the name 'xenpv' because it doesn't convey the fact that some of
> the HW may be emulated rather than para-virtualized. In fact one may only want
> to use for emulating devices.
> 
> Potential name would be 'xen-arm' or re-using 'virt' but with 'accel=xen' to
> select a Xen layout.

The benefit of 'xenpv' is that it doesn't require any changes to libxl.
It is even backward compatible so it could be used with an older version
of Xen/libxl. Backward compatibility aside, if we come up with a
different name then we'll need changes to libxl and to manage those
changes. For instance, if we use 'xen-arm' that would mean we would need
to handle per-arch QEMU machine names.
Julien Grall Oct. 18, 2022, 8:24 a.m. UTC | #3
Hi Stefano,

On 18/10/2022 02:26, Stefano Stabellini wrote:
> On Sun, 16 Oct 2022, Julien Grall wrote:
>> Hi,
>>
>> There seem to be some missing patches on xen-devel (including the cover
>> letter). Is that expected?
>>
>> On 15/10/2022 06:07, Vikram Garhwal wrote:
>>> Add a new machine xenpv which creates a IOREQ server to register/connect
>>> with
>>> Xen Hypervisor.
>>
>> I don't like the name 'xenpv' because it doesn't convey the fact that some of
>> the HW may be emulated rather than para-virtualized. In fact one may only want
>> to use for emulating devices.
>>
>> Potential name would be 'xen-arm' or re-using 'virt' but with 'accel=xen' to
>> select a Xen layout.
> 
> The benefit of 'xenpv' is that it doesn't require any changes to libxl.

I am quite surprised. Looking at the code, it seems to work more by 
chance than it is intentional as the code is gated by 
libxl__need_xenpv_qemu(). So it would not start if there were no 
emulated devices.

> It is even backward compatible so it could be used with an older version
> of Xen/libxl.
We don't really gain much here. IOREQ is a tech preview and anyone that 
wants to try it should really use the latest Xen.

> Backward compatibility aside, if we come up with a
> different name then we'll need changes to libxl and to manage those
> changes. For instance, if we use 'xen-arm' that would mean we would need
> to handle per-arch QEMU machine names.

Right, so the main argument here is for simplicity in libxl
Looking at how 'xenpv' is built, this is really expected to deal with PV 
backend rather than emulated device. I do expect some changes as we go 
along to be able to add emulated device.

Furthermore, libxl is not the only toolstack out. So I am not convinced 
this is a good argument to keep the name the same.

Cheers,
Stefano Stabellini Oct. 19, 2022, 12:15 a.m. UTC | #4
On Tue, 18 Oct 2022, Julien Grall wrote:
> On 18/10/2022 02:26, Stefano Stabellini wrote:
> > On Sun, 16 Oct 2022, Julien Grall wrote:
> > > Hi,
> > > 
> > > There seem to be some missing patches on xen-devel (including the cover
> > > letter). Is that expected?
> > > 
> > > On 15/10/2022 06:07, Vikram Garhwal wrote:
> > > > Add a new machine xenpv which creates a IOREQ server to register/connect
> > > > with
> > > > Xen Hypervisor.
> > > 
> > > I don't like the name 'xenpv' because it doesn't convey the fact that some
> > > of
> > > the HW may be emulated rather than para-virtualized. In fact one may only
> > > want
> > > to use for emulating devices.
> > > 
> > > Potential name would be 'xen-arm' or re-using 'virt' but with 'accel=xen'
> > > to
> > > select a Xen layout.
> > 
> > The benefit of 'xenpv' is that it doesn't require any changes to libxl.
> 
> I am quite surprised. Looking at the code, it seems to work more by chance
> than it is intentional as the code is gated by libxl__need_xenpv_qemu(). So it
> would not start if there were no emulated devices.
> 
> > It is even backward compatible so it could be used with an older version
> > of Xen/libxl.
> We don't really gain much here. IOREQ is a tech preview and anyone that wants
> to try it should really use the latest Xen.

I think that's fair.


> > Backward compatibility aside, if we come up with a
> > different name then we'll need changes to libxl and to manage those
> > changes. For instance, if we use 'xen-arm' that would mean we would need
> > to handle per-arch QEMU machine names.
> 
> Right, so the main argument here is for simplicity in libxl

Yeah


> Looking at how 'xenpv' is built, this is really expected to deal with PV
> backend rather than emulated device. I do expect some changes as we go along
> to be able to add emulated device.
> 
> Furthermore, libxl is not the only toolstack out. So I am not convinced this
> is a good argument to keep the name the same.

Let's think some more about the naming strategy. From a QEMU point of
view we could choose any name we like (Vikram please confirm), the issue
is really on the libxl side.

Today libxl understands two QEMU "machines": xenpv and xenfv
(pc,accel=xen is the same as xenfv, I'll use xenfv in this email for
simplicity).

xenpv is for PV guests and only provides PV backends, no emulation. It
is used on both ARM and x86.

xenfv is only used on x86, and it is for HVM guests, with a full set of
emulated hardware (PIIX3, etc.).

The purpose of this series is to introduce a QEMU machine that:
- works on ARM (but could maybe work on other archs as a stretch goal)
- provides PV backends
- no emulated devices by default
- also can emulate selected devices on request

Certainly it is not xenfv or pc,accel=xen because they would with more
emulation by default. This is more like "xenpvh": an extension to PV
with also the capability of emulating one device as requested. It is not
intended to emulate a full PC and doesn't do that by default like xenfv.

If/When x86 PVH gains the ability to use QEMU as IOREQ server, I would
imagine it would run a QEMU machine similar to this one.

This is also how I would imagine it would get integrated in libxl: as a
xenpv + individual emulated devices. Not as HVM for ARM. The other QEMU
command line arguments are inline with the xenpv command line arguments
rather than xenfv command line arguments. This is why the libxl changes
are small to zero to make it work today.

So, I think the following options work:

a) call it "xenpv" because it is an extension of the old xenpv machine
b) call it "xenpvh" because it is PV + few individual emulated devices

If we call it xenpv there are fewer changes in libxl. If we call it
xenpvh there are more changes in libxl but we can distinguish xenpv from
xenpvh (I don't see why we need it right now, but I could imagine it
could turn out useful in the future.)

I would stay away from arch-specific machine names because it will make
it harder on the libxl side without immediate benefits.
Julien Grall Oct. 19, 2022, 9:49 a.m. UTC | #5
Hi Stefano,

On 19/10/2022 01:15, Stefano Stabellini wrote:
> On Tue, 18 Oct 2022, Julien Grall wrote:
>> On 18/10/2022 02:26, Stefano Stabellini wrote:
>>> On Sun, 16 Oct 2022, Julien Grall wrote:
>>>> Hi,
>>>>
>>>> There seem to be some missing patches on xen-devel (including the cover
>>>> letter). Is that expected?
>>>>
>>>> On 15/10/2022 06:07, Vikram Garhwal wrote:
>>>>> Add a new machine xenpv which creates a IOREQ server to register/connect
>>>>> with
>>>>> Xen Hypervisor.
>>>>
>>>> I don't like the name 'xenpv' because it doesn't convey the fact that some
>>>> of
>>>> the HW may be emulated rather than para-virtualized. In fact one may only
>>>> want
>>>> to use for emulating devices.
>>>>
>>>> Potential name would be 'xen-arm' or re-using 'virt' but with 'accel=xen'
>>>> to
>>>> select a Xen layout.
>>>
>>> The benefit of 'xenpv' is that it doesn't require any changes to libxl.
>>
>> I am quite surprised. Looking at the code, it seems to work more by chance
>> than it is intentional as the code is gated by libxl__need_xenpv_qemu(). So it
>> would not start if there were no emulated devices.
>>
>>> It is even backward compatible so it could be used with an older version
>>> of Xen/libxl.
>> We don't really gain much here. IOREQ is a tech preview and anyone that wants
>> to try it should really use the latest Xen.
> 
> I think that's fair.
> 
> 
>>> Backward compatibility aside, if we come up with a
>>> different name then we'll need changes to libxl and to manage those
>>> changes. For instance, if we use 'xen-arm' that would mean we would need
>>> to handle per-arch QEMU machine names.
>>
>> Right, so the main argument here is for simplicity in libxl
> 
> Yeah
> 
> 
>> Looking at how 'xenpv' is built, this is really expected to deal with PV
>> backend rather than emulated device. I do expect some changes as we go along
>> to be able to add emulated device.
>>
>> Furthermore, libxl is not the only toolstack out. So I am not convinced this
>> is a good argument to keep the name the same.
> 
> Let's think some more about the naming strategy. From a QEMU point of
> view we could choose any name we like (Vikram please confirm), the issue
> is really on the libxl side.
> 
> Today libxl understands two QEMU "machines": xenpv and xenfv
> (pc,accel=xen is the same as xenfv, I'll use xenfv in this email for
> simplicity).
> 
> xenpv is for PV guests and only provides PV backends, no emulation. It
> is used on both ARM and x86.
> 
> xenfv is only used on x86, and it is for HVM guests, with a full set of
> emulated hardware (PIIX3, etc.).
> 
> The purpose of this series is to introduce a QEMU machine that:
> - works on ARM (but could maybe work on other archs as a stretch goal)
> - provides PV backends
> - no emulated devices by default
> - also can emulate selected devices on request
> 
> Certainly it is not xenfv or pc,accel=xen because they would with more
> emulation by default. This is more like "xenpvh": an extension to PV
> with also the capability of emulating one device as requested. It is not
> intended to emulate a full PC and doesn't do that by default like xenfv.

The definition of "full PC" is not very clear for me. Unlike x86, Arm 
doesn't have legacy devices that needs to be emulated. So technically, 
if we emulated one network card and one block device, then we would be 
able potentially be able to boot an unaware OS on Xen on Arm. This would 
be the same as if you passthrough-ed the two devices.

In the past, I have seen interest to boot OS like Windows OS/iOS on Arm. 
I do expect that the addition of a Xen platform in QEMU will lead to 
another increase of the interest because we could expose anything to the 
VM. Although, it might need some tweak in Xen to allow more dynamic 
layout just in case an OS doesn't discover dynamically devices.

> 
> If/When x86 PVH gains the ability to use QEMU as IOREQ server, I would
> imagine it would run a QEMU machine similar to this one.
To me it would sounds odd to add emulated devices in the 'xenpv' because 
they would only work for PVH domain. AFAIK, QEMU doesn't know whether a 
domain is PV or PVH. So we would solely rely on IOREQ to return an error.

> 
> This is also how I would imagine it would get integrated in libxl: as a
> xenpv + individual emulated devices. Not as HVM for ARM. The other QEMU
> command line arguments are inline with the xenpv command line arguments
> rather than xenfv command line arguments. This is why the libxl changes
> are small to zero to make it work today.
> 
> So, I think the following options work:
> 
> a) call it "xenpv" because it is an extension of the old xenpv machine
> b) call it "xenpvh" because it is PV + few individual emulated devices
> 
> If we call it xenpv there are fewer changes in libxl. If we call it
> xenpvh there are more changes in libxl but we can distinguish xenpv from
> xenpvh (I don't see why we need it right now, but I could imagine it
> could turn out useful in the future.)

IMHO, we need to plan for the future.

> 
> I would stay away from arch-specific machine names because it will make
> it harder on the libxl side without immediate benefits.

If the name is the only change, then I would expect this could be done 
with a per-arch define. So that would be a few lines change maximum.

Cheers,
Alex Bennée Oct. 27, 2022, 8:02 a.m. UTC | #6
Vikram Garhwal <vikram.garhwal@amd.com> writes:

<snip>
> 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 &

<snip>
> +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);

I'm not sure what has gone wrong here but I'm getting:

  ../../hw/arm/xen_arm.c: In function ‘xen_enable_tpm’:
  ../../hw/arm/xen_arm.c:120:32: error: ‘GUEST_TPM_BASE’ undeclared (first use in this function); did you mean ‘GUEST_RAM_BASE’?
    120 |     sysbus_mmio_map(busdev, 0, GUEST_TPM_BASE);
        |                                ^~~~~~~~~~~~~~
        |                                GUEST_RAM_BASE
  ../../hw/arm/xen_arm.c:120:32: note: each undeclared identifier is reported only once for each function it appears in

In my cross build:

  # Configured with: '../../configure' '--disable-docs' '--target-list=aarch64-softmmu' '--disable-kvm' '--enable-xen' '--disable-opengl' '--disable-libudev' '--enable-tpm' '--disable-xen-pci-passthrough' '--cross-prefix=aarch64-linux-gnu-' '--skip-meson'

which makes me wonder if this is a configure failure or a confusion
about being able to have host swtpm implementations during emulation but
needing target tpm for Xen?
Alex Bennée Oct. 27, 2022, 8:37 a.m. UTC | #7
Julien Grall <julien@xen.org> writes:

> Hi,
>
> There seem to be some missing patches on xen-devel (including the
> cover letter). Is that expected?
>
> On 15/10/2022 06:07, Vikram Garhwal wrote:
>> Add a new machine xenpv which creates a IOREQ server to register/connect with
>> Xen Hypervisor.
>
> I don't like the name 'xenpv' because it doesn't convey the fact that
> some of the HW may be emulated rather than para-virtualized. In fact
> one may only want to use for emulating devices.
>
> Potential name would be 'xen-arm' or re-using 'virt' but with
> 'accel=xen' to select a Xen layout.

I don't think you can re-use the machine name and select by accelerator
because the virt machine does quite a lot of other stuff this model
doesn't support. However I've been calling this concept "xen-virt" or
maybe the explicit "xen-virtio" because that is what it is targeting.
Alex Bennée Oct. 27, 2022, 8:40 a.m. UTC | #8
Vikram Garhwal <vikram.garhwal@amd.com> writes:

> Add a new machine xenpv which creates a IOREQ server to register/connect with
> Xen Hypervisor.
>
<snip>
> 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.
<snip>
> +
> +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;
> +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);

This needs #ifdef CONFIG_TPM because while doing --disable-tpm to try
and get the cross build working it then fails with:

../../hw/arm/xen_arm.c: In function ‘xen_arm_machine_class_init’:
../../hw/arm/xen_arm.c:148:48: error: ‘TYPE_TPM_TIS_SYSBUS’ undeclared (first use in this function)
  148 |     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
      |                                                ^~~~~~~~~~~~~~~~~~~
../../hw/arm/xen_arm.c:148:48: note: each undeclared identifier is reported only once for each function it appears in
Julien Grall Oct. 28, 2022, 5:57 p.m. UTC | #9
Hi,

On 27/10/2022 09:02, Alex Bennée wrote:
> 
> Vikram Garhwal <vikram.garhwal@amd.com> writes:
> 
> <snip>
>> 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 &
> 
> <snip>
>> +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);
> 
> I'm not sure what has gone wrong here but I'm getting:
> 
>    ../../hw/arm/xen_arm.c: In function ‘xen_enable_tpm’:
>    ../../hw/arm/xen_arm.c:120:32: error: ‘GUEST_TPM_BASE’ undeclared (first use in this function); did you mean ‘GUEST_RAM_BASE’?
>      120 |     sysbus_mmio_map(busdev, 0, GUEST_TPM_BASE);
>          |                                ^~~~~~~~~~~~~~
>          |                                GUEST_RAM_BASE
>    ../../hw/arm/xen_arm.c:120:32: note: each undeclared identifier is reported only once for each function it appears in
> 
> In my cross build:
> 
>    # Configured with: '../../configure' '--disable-docs' '--target-list=aarch64-softmmu' '--disable-kvm' '--enable-xen' '--disable-opengl' '--disable-libudev' '--enable-tpm' '--disable-xen-pci-passthrough' '--cross-prefix=aarch64-linux-gnu-' '--skip-meson'
> 
> which makes me wonder if this is a configure failure or a confusion
> about being able to have host swtpm implementations during emulation but
> needing target tpm for Xen?

I was also wondering where is that value come from. Note that the 
memory/IRQ layout exposed to the guest is not stable.

Are we expecting the user to rebuild QEMU for every Xen versions (or 
possibly every guest if we ever allow dynamic layout in Xen)?

Cheers,
Oleksandr Tyshchenko Oct. 28, 2022, 7:05 p.m. UTC | #10
On Fri, Oct 28, 2022 at 8:58 PM Julien Grall <julien@xen.org> wrote:

> Hi,
>

Hello all.

[sorry for the possible format issues]



>
> On 27/10/2022 09:02, Alex Bennée wrote:
> >
> > Vikram Garhwal <vikram.garhwal@amd.com> writes:
> >
> > <snip>
> >> 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 &
> >
> > <snip>
> >> +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);
> >
> > I'm not sure what has gone wrong here but I'm getting:
> >
> >    ../../hw/arm/xen_arm.c: In function ‘xen_enable_tpm’:
> >    ../../hw/arm/xen_arm.c:120:32: error: ‘GUEST_TPM_BASE’ undeclared
> (first use in this function); did you mean ‘GUEST_RAM_BASE’?
> >      120 |     sysbus_mmio_map(busdev, 0, GUEST_TPM_BASE);
> >          |                                ^~~~~~~~~~~~~~
> >          |                                GUEST_RAM_BASE
> >    ../../hw/arm/xen_arm.c:120:32: note: each undeclared identifier is
> reported only once for each function it appears in
> >
> > In my cross build:
> >
> >    # Configured with: '../../configure' '--disable-docs'
> '--target-list=aarch64-softmmu' '--disable-kvm' '--enable-xen'
> '--disable-opengl' '--disable-libudev' '--enable-tpm'
> '--disable-xen-pci-passthrough' '--cross-prefix=aarch64-linux-gnu-'
> '--skip-meson'
> >
> > which makes me wonder if this is a configure failure or a confusion
> > about being able to have host swtpm implementations during emulation but
> > needing target tpm for Xen?
>
> I was also wondering where is that value come from. Note that the
> memory/IRQ layout exposed to the guest is not stable.
>
> Are we expecting the user to rebuild QEMU for every Xen versions (or
> possibly every guest if we ever allow dynamic layout in Xen)?
>


This doesn't sound ideal.

I am wondering what would be the correct way here assuming that we would
likely need to have more such information in place for supporting more
use-cases...
For instance, the PCI host bridge emulation in Qemu. Xen toolstack (another
software layer) generates device-tree for the guest, so creates PCI Host
bridge node by using reserved regions from Guest OS interface (arch-arm.h):
- GUEST_VPCI_MEM_ADDR (GUEST_VPCI_MEM_SIZE)
- GUEST_VPCI_ECAM_BASE (GUEST_VPCI_ECAM_SIZE)
- GUEST_VPCI_PREFETCH_MEM_ADDR (GUEST_VPCI_PREFETCH_MEM_SIZE)
https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libs/light/libxl_arm.c;h=2a5e93c28403738779863aded31d2df3ba72f8c0;hb=HEAD#l833

Here in Qemu when creating a PCI Host bridge we would need to use exactly
the same reserved regions which toolstack writes in the corresponding
device-tree node. So how to tell Qemu about them?
1. Introduce new cmd line arguments?
2. Using Xenstore?
3. Anything else?

I am afraid this would be related to every device that we want to emulate
in Qemu and for which the toolstack needs to generate device-tree node by
using something defined with GUEST_*, unless I really missed something.



>
> Cheers,
>
> --
> Julien Grall
>
>
Vikram Garhwal Dec. 2, 2022, 3:24 a.m. UTC | #11
Hi Julien,

From: Julien Grall <julien@xen.org>
Date: Sunday, October 16, 2022 at 10:48 AM
To: Garhwal, Vikram <vikram.garhwal@amd.com>, qemu-devel@nongnu.org <qemu-devel@nongnu.org>
Cc: Stabellini, Stefano <stefano.stabellini@amd.com>, Peter Maydell <peter.maydell@linaro.org>, Stefano Stabellini <sstabellini@kernel.org>, Anthony Perard <anthony.perard@citrix.com>, Paul Durrant <paul@xen.org>, open list:ARM TCG CPUs <qemu-arm@nongnu.org>, open list:X86 Xen CPUs <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH v1 10/12] hw/arm: introduce xenpv machine
Hi,

There seem to be some missing patches on xen-devel (including the cover
letter). Is that expected?
Not sure what went wrong there. I can see all of these on QEMU-devel. Perhaps xen-devel is not in maintainer’s list for all the xen files?


On 15/10/2022 06:07, Vikram Garhwal wrote:
> Add a new machine xenpv which creates a IOREQ server to register/connect with
> Xen Hypervisor.

I don't like the name 'xenpv' because it doesn't convey the fact that
some of the HW may be emulated rather than para-virtualized. In fact one
may only want to use for emulating devices.

Potential name would be 'xen-arm' or re-using 'virt' but with
'accel=xen' to select a Xen layout.

>
> Xen IOREQ connection expect the TARGET_PAGE_SIZE to 4096, and the xenpv machine
> on ARM will have no CPU definitions. We need to define TARGET_PAGE_SIZE
> appropriately ourselves.
>
> 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 &

I see patches for QEMU but not Xen. How can this be tested with existing
Xen? Will libxl ever create QEMU?
Will send the patch for libxl Xen separately.


[...]

> +static int xen_init_ioreq(XenIOState *state, unsigned int max_cpus)
> +{
> +    xen_dmod = xendevicemodel_open(0, 0);
> +    xen_xc = xc_interface_open(0, 0, 0);
> +
> +    if (xen_xc == NULL) {

You are checking xen_xc but not xen_dmod. Why?

> +        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);
> +
> +    xenstore_record_dm_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);

I can't find where GUEST_TPM_BASE is defined. But then the guest memory
layout is not expected to be stable. With your current approach, it
means QEMU would need to be rebuilt for every Xen version. Is it what we
want?
I cannot think of better way to do this. Either we add the the def here or rebuild it if GUEST_TPM_BASE changes for each xen version.



> +
> +    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);
> +
> +    if (xen_init_ioreq(xam->state, machine->smp.cpus)) {
> +        return;

In another patch, you said the IOREQ would be optional. IHMO, I think
this is a bad idea to register it by default because one may only want
to use PV drivers. Registering IOREQ will add unnecessary overhead in Xen.

Furthermore, it means that someone selecting TPM but Xen is not built
with CONFIG_IOREQ=y (BTW This is still a tech preview but there are
security holes on Arm...) will not get an error. Instead, the OS will
until it crashes when trying to access the TPM.

Overall I think it would be better if IOREQ is only registered when a
device requires (like TPM) it *and* throw an error if there is a problem
during the initialization.


> +    } > +
> +    xen_enable_tpm();
> +
> +    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;
> +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);

Shouldn't this be protected with #ifdef CONFIG_TPM?

> +}
> +
> +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..f645dfec28
> --- /dev/null
> +++ b/include/hw/arm/xen_arch_hvm.h
> @@ -0,0 +1,12 @@
> +#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);
> +
> +#undef TARGET_PAGE_SIZE

I am a bit puzzled with this #undef. In the commit message you said that
there will be no CPU definition. So the implications is this should not
be defined.

If it is defined, then what guarantees that all the source will use the
correct value?


> +#define TARGET_PAGE_SIZE 4096

It would be better to use XC_PAGE_SIZE (or similar) rather than
hardcoding it.
Corrected this and sent a v2.


> +#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

Cheers,

--
Julien Grall
Julien Grall Dec. 2, 2022, 9:53 a.m. UTC | #12
On 02/12/2022 03:24, Garhwal, Vikram wrote:
> Hi Julien,

Hi Vikram,

I am having trouble to differentiate your answers from my remark. For 
instance...


> From: Julien Grall <julien@xen.org>
> Date: Sunday, October 16, 2022 at 10:48 AM
> To: Garhwal, Vikram <vikram.garhwal@amd.com>, qemu-devel@nongnu.org <qemu-devel@nongnu.org>
> Cc: Stabellini, Stefano <stefano.stabellini@amd.com>, Peter Maydell <peter.maydell@linaro.org>, Stefano Stabellini <sstabellini@kernel.org>, Anthony Perard <anthony.perard@citrix.com>, Paul Durrant <paul@xen.org>, open list:ARM TCG CPUs <qemu-arm@nongnu.org>, open list:X86 Xen CPUs <xen-devel@lists.xenproject.org>
> Subject: Re: [PATCH v1 10/12] hw/arm: introduce xenpv machine
> Hi,
> 
> There seem to be some missing patches on xen-devel (including the cover
> letter). Is that expected?
> Not sure what went wrong there. I can see all of these on QEMU-devel. Perhaps xen-devel is not in maintainer’s list for all the xen files?
> 
> 
> On 15/10/2022 06:07, Vikram Garhwal wrote:
>> Add a new machine xenpv which creates a IOREQ server to register/connect with
>> Xen Hypervisor.
> 
> I don't like the name 'xenpv' because it doesn't convey the fact that
> some of the HW may be emulated rather than para-virtualized. In fact one
> may only want to use for emulating devices.
> 
> Potential name would be 'xen-arm' or re-using 'virt' but with
> 'accel=xen' to select a Xen layout.
> 
>>
>> Xen IOREQ connection expect the TARGET_PAGE_SIZE to 4096, and the xenpv machine
>> on ARM will have no CPU definitions. We need to define TARGET_PAGE_SIZE
>> appropriately ourselves.
>>
>> 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 &
> 
> I see patches for QEMU but not Xen. How can this be tested with existing
> Xen? Will libxl ever create QEMU?
> Will send the patch for libxl Xen separately.

... the first two lines are my remarks and the 3rd is your answer. Can 
you configure your e-mail client to do proper quoting?

[...]

>> +    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);
> 
> I can't find where GUEST_TPM_BASE is defined. But then the guest memory
> layout is not expected to be stable. With your current approach, it
> means QEMU would need to be rebuilt for every Xen version. Is it what we
> want?
> I cannot think of better way to do this. Either we add the the def here or rebuild it if GUEST_TPM_BASE changes for each xen version.

The alternative would be to specify the address on the QEMU command 
line. The advantage is you could build a system where each guests have 
different layout.

Cheers,
diff mbox series

Patch

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..0b900314cc
--- /dev/null
+++ b/hw/arm/xen_arm.c
@@ -0,0 +1,163 @@ 
+/*
+ * 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)
+{
+}
+
+static int xen_init_ioreq(XenIOState *state, unsigned int max_cpus)
+{
+    xen_dmod = xendevicemodel_open(0, 0);
+    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);
+
+    xenstore_record_dm_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);
+
+    if (xen_init_ioreq(xam->state, machine->smp.cpus)) {
+        return;
+    }
+
+    xen_enable_tpm();
+
+    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;
+    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
+}
+
+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..f645dfec28
--- /dev/null
+++ b/include/hw/arm/xen_arch_hvm.h
@@ -0,0 +1,12 @@ 
+#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);
+
+#undef TARGET_PAGE_SIZE
+#define TARGET_PAGE_SIZE 4096
+#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