|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] RE: [RFC PATCH V1 01/12] hvm/ioreq: Make x86's IOREQ feature common
> -----Original Message-----
> From: Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>
> Sent: 03 August 2020 19:21
> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>; Jan Beulich
> <jbeulich@xxxxxxxx>; Andrew
> Cooper <andrew.cooper3@xxxxxxxxxx>; Wei Liu <wl@xxxxxxx>; Roger Pau Monné
> <roger.pau@xxxxxxxxxx>;
> George Dunlap <george.dunlap@xxxxxxxxxx>; Ian Jackson
> <ian.jackson@xxxxxxxxxxxxx>; Julien Grall
> <julien@xxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>; Paul Durrant
> <paul@xxxxxxx>; Jun
> Nakajima <jun.nakajima@xxxxxxxxx>; Kevin Tian <kevin.tian@xxxxxxxxx>; Tim
> Deegan <tim@xxxxxxx>; Julien
> Grall <julien.grall@xxxxxxx>
> Subject: [RFC PATCH V1 01/12] hvm/ioreq: Make x86's IOREQ feature common
>
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
>
> As a lot of x86 code can be re-used on Arm later on, this patch
> splits IOREQ support into common and arch specific parts.
>
> This support is going to be used on Arm to be able run device
> emulator outside of Xen hypervisor.
>
> Please note, this is a split/cleanup of Julien's PoC:
> "Add support for Guest IO forwarding to a device emulator"
>
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> ---
> xen/arch/x86/Kconfig | 1 +
> xen/arch/x86/hvm/dm.c | 2 +-
> xen/arch/x86/hvm/emulate.c | 2 +-
> xen/arch/x86/hvm/hvm.c | 2 +-
> xen/arch/x86/hvm/io.c | 2 +-
> xen/arch/x86/hvm/ioreq.c | 1431
> +--------------------------------------
> xen/arch/x86/hvm/stdvga.c | 2 +-
> xen/arch/x86/hvm/vmx/realmode.c | 1 +
> xen/arch/x86/hvm/vmx/vvmx.c | 2 +-
> xen/arch/x86/mm.c | 2 +-
> xen/arch/x86/mm/shadow/common.c | 2 +-
> xen/common/Kconfig | 3 +
> xen/common/Makefile | 1 +
> xen/common/hvm/Makefile | 1 +
> xen/common/hvm/ioreq.c | 1430 ++++++++++++++++++++++++++++++++++++++
> xen/include/asm-x86/hvm/ioreq.h | 45 +-
> xen/include/asm-x86/hvm/vcpu.h | 7 -
> xen/include/xen/hvm/ioreq.h | 89 +++
> 18 files changed, 1575 insertions(+), 1450 deletions(-)
> create mode 100644 xen/common/hvm/Makefile
> create mode 100644 xen/common/hvm/ioreq.c
> create mode 100644 xen/include/xen/hvm/ioreq.h
You need to adjust the MAINTAINERS file since there will now be common 'I/O
EMULATION' code. Since I wrote most of ioreq.c, please retain me as a
maintainer of the common code.
[snip]
> @@ -1528,13 +143,19 @@ static int hvm_access_cf8(
> return X86EMUL_UNHANDLEABLE;
> }
>
> -void hvm_ioreq_init(struct domain *d)
> +void arch_hvm_ioreq_init(struct domain *d)
> {
> spin_lock_init(&d->arch.hvm.ioreq_server.lock);
>
> register_portio_handler(d, 0xcf8, 4, hvm_access_cf8);
> }
>
> +void arch_hvm_ioreq_destroy(struct domain *d)
> +{
> + if ( !relocate_portio_handler(d, 0xcf8, 0xcf8, 4) )
> + return;
There's not really a lot of point in this. I think an empty function here would
be ok.
> +}
> +
> /*
> * Local variables:
> * mode: C
[snip]
> +struct hvm_ioreq_server *hvm_select_ioreq_server(struct domain *d,
> + ioreq_t *p)
> +{
> + struct hvm_ioreq_server *s;
> + uint8_t type;
> + uint64_t addr;
> + unsigned int id;
> +
> + if ( p->type != IOREQ_TYPE_COPY && p->type != IOREQ_TYPE_PIO )
> + return NULL;
> +
> + hvm_get_ioreq_server_range_type(d, p, &type, &addr);
Looking at this, I think it would make more sense to fold the check of p->type
into hvm_get_ioreq_server_range_type() and have it return success/failure.
> +
> + FOR_EACH_IOREQ_SERVER(d, id, s)
> + {
> + struct rangeset *r;
> +
> + if ( !s->enabled )
> + continue;
> +
> + r = s->range[type];
> +
> + switch ( type )
> + {
> + unsigned long start, end;
> +
> + case XEN_DMOP_IO_RANGE_PORT:
> + start = addr;
> + end = start + p->size - 1;
> + if ( rangeset_contains_range(r, start, end) )
> + return s;
> +
> + break;
> +
> + case XEN_DMOP_IO_RANGE_MEMORY:
> + start = hvm_mmio_first_byte(p);
> + end = hvm_mmio_last_byte(p);
> +
> + if ( rangeset_contains_range(r, start, end) )
> + return s;
> +
> + break;
> +
> + case XEN_DMOP_IO_RANGE_PCI:
> + if ( rangeset_contains_singleton(r, addr >> 32) )
> + {
> + p->type = IOREQ_TYPE_PCI_CONFIG;
> + p->addr = addr;
> + return s;
> + }
> +
> + break;
> + }
> + }
> +
> + return NULL;
> +}
[snip]
> diff --git a/xen/include/xen/hvm/ioreq.h b/xen/include/xen/hvm/ioreq.h
> new file mode 100644
> index 0000000..40b7b5e
> --- /dev/null
> +++ b/xen/include/xen/hvm/ioreq.h
> @@ -0,0 +1,89 @@
> +/*
> + * hvm.h: Hardware virtual machine assist interface definitions.
> + *
> + * Copyright (c) 2016 Citrix Systems Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> with
> + * this program; If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __HVM_IOREQ_H__
> +#define __HVM_IOREQ_H__
> +
> +#include <xen/sched.h>
> +
> +#include <asm/hvm/ioreq.h>
> +
> +#define GET_IOREQ_SERVER(d, id) \
> + (d)->arch.hvm.ioreq_server.server[id]
> +
> +static inline struct hvm_ioreq_server *get_ioreq_server(const struct domain
> *d,
> + unsigned int id)
> +{
> + if ( id >= MAX_NR_IOREQ_SERVERS )
> + return NULL;
> +
> + return GET_IOREQ_SERVER(d, id);
> +}
> +
> +static inline bool hvm_ioreq_needs_completion(const ioreq_t *ioreq)
> +{
> + return ioreq->state == STATE_IOREQ_READY &&
> + !ioreq->data_is_ptr &&
> + (ioreq->type != IOREQ_TYPE_PIO || ioreq->dir != IOREQ_WRITE);
> +}
I don't think having this in common code is correct. The short-cut of not
completing PIO reads seems somewhat x86 specific. Does ARM even have the
concept of PIO?
Paul
> +
> +bool hvm_io_pending(struct vcpu *v);
> +bool handle_hvm_io_completion(struct vcpu *v);
> +bool is_ioreq_server_page(struct domain *d, const struct page_info *page);
> +
> +int hvm_create_ioreq_server(struct domain *d, int bufioreq_handling,
> + ioservid_t *id);
> +int hvm_destroy_ioreq_server(struct domain *d, ioservid_t id);
> +int hvm_get_ioreq_server_info(struct domain *d, ioservid_t id,
> + unsigned long *ioreq_gfn,
> + unsigned long *bufioreq_gfn,
> + evtchn_port_t *bufioreq_port);
> +int hvm_get_ioreq_server_frame(struct domain *d, ioservid_t id,
> + unsigned long idx, mfn_t *mfn);
> +int hvm_map_io_range_to_ioreq_server(struct domain *d, ioservid_t id,
> + uint32_t type, uint64_t start,
> + uint64_t end);
> +int hvm_unmap_io_range_from_ioreq_server(struct domain *d, ioservid_t id,
> + uint32_t type, uint64_t start,
> + uint64_t end);
> +int hvm_set_ioreq_server_state(struct domain *d, ioservid_t id,
> + bool enabled);
> +
> +int hvm_all_ioreq_servers_add_vcpu(struct domain *d, struct vcpu *v);
> +void hvm_all_ioreq_servers_remove_vcpu(struct domain *d, struct vcpu *v);
> +void hvm_destroy_all_ioreq_servers(struct domain *d);
> +
> +struct hvm_ioreq_server *hvm_select_ioreq_server(struct domain *d,
> + ioreq_t *p);
> +int hvm_send_ioreq(struct hvm_ioreq_server *s, ioreq_t *proto_p,
> + bool buffered);
> +unsigned int hvm_broadcast_ioreq(ioreq_t *p, bool buffered);
> +
> +void hvm_ioreq_init(struct domain *d);
> +
> +#endif /* __HVM_IOREQ_H__ */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> --
> 2.7.4
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |