From: Paul Durrant <xadimgnik@gmail.com>
To: "'Oleksandr'" <olekstysh@gmail.com>, <xen-devel@lists.xenproject.org>
Cc: "'Kevin Tian'" <kevin.tian@intel.com>,
"'Stefano Stabellini'" <sstabellini@kernel.org>,
"'Julien Grall'" <julien@xen.org>,
"'Jun Nakajima'" <jun.nakajima@intel.com>,
"'Wei Liu'" <wl@xen.org>,
"'Andrew Cooper'" <andrew.cooper3@citrix.com>,
"'Ian Jackson'" <ian.jackson@eu.citrix.com>,
"'George Dunlap'" <george.dunlap@citrix.com>,
"'Tim Deegan'" <tim@xen.org>,
"'Oleksandr Tyshchenko'" <oleksandr_tyshchenko@epam.com>,
"'Julien Grall'" <julien.grall@arm.com>,
"'Jan Beulich'" <jbeulich@suse.com>,
"'Roger Pau Monné'" <roger.pau@citrix.com>
Subject: RE: [RFC PATCH V1 01/12] hvm/ioreq: Make x86's IOREQ feature common
Date: Tue, 4 Aug 2020 12:23:40 +0100 [thread overview]
Message-ID: <002101d66a51$b4c59070$1e50b150$@xen.org> (raw)
In-Reply-To: <9f83a7ed-ca97-449f-c7b9-a1140644abe9@gmail.com>
> -----Original Message-----
> From: Oleksandr <olekstysh@gmail.com>
> Sent: 04 August 2020 12:10
> To: paul@xen.org; xen-devel@lists.xenproject.org
> Cc: 'Oleksandr Tyshchenko' <oleksandr_tyshchenko@epam.com>; 'Jan Beulich' <jbeulich@suse.com>; 'Andrew
> Cooper' <andrew.cooper3@citrix.com>; 'Wei Liu' <wl@xen.org>; 'Roger Pau Monné' <roger.pau@citrix.com>;
> 'George Dunlap' <george.dunlap@citrix.com>; 'Ian Jackson' <ian.jackson@eu.citrix.com>; 'Julien Grall'
> <julien@xen.org>; 'Stefano Stabellini' <sstabellini@kernel.org>; 'Jun Nakajima'
> <jun.nakajima@intel.com>; 'Kevin Tian' <kevin.tian@intel.com>; 'Tim Deegan' <tim@xen.org>; 'Julien
> Grall' <julien.grall@arm.com>
> Subject: Re: [RFC PATCH V1 01/12] hvm/ioreq: Make x86's IOREQ feature common
>
>
> On 04.08.20 10:45, Paul Durrant wrote:
>
> Hi Paul
>
> >> -----Original Message-----
> >> From: Oleksandr Tyshchenko <olekstysh@gmail.com>
> >> Sent: 03 August 2020 19:21
> >> To: xen-devel@lists.xenproject.org
> >> Cc: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>; Jan Beulich <jbeulich@suse.com>; Andrew
> >> Cooper <andrew.cooper3@citrix.com>; Wei Liu <wl@xen.org>; Roger Pau Monné <roger.pau@citrix.com>;
> >> George Dunlap <george.dunlap@citrix.com>; Ian Jackson <ian.jackson@eu.citrix.com>; Julien Grall
> >> <julien@xen.org>; Stefano Stabellini <sstabellini@kernel.org>; Paul Durrant <paul@xen.org>; Jun
> >> Nakajima <jun.nakajima@intel.com>; Kevin Tian <kevin.tian@intel.com>; Tim Deegan <tim@xen.org>;
> Julien
> >> Grall <julien.grall@arm.com>
> >> Subject: [RFC PATCH V1 01/12] hvm/ioreq: Make x86's IOREQ feature common
> >>
> >> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> >>
> >> 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@arm.com>
> >> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> >> ---
> >> 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.
>
> Oh, I completely forgot about MAINTAINERS file. Sure, I will update file
> and retain you.
>
>
> >
> > [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.
>
> ok
>
>
> >> +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.
>
> ok, will update.
>
>
> > 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?
>
> I am not 100% sure here, but it seems that doesn't have.
>
> Shall I make hvm_ioreq_needs_completion() per arch? Arm variant would
> have the same implementation, but without "ioreq->type !=
> IOREQ_TYPE_PIO" check...
>
With your series applied, does any common code actually call hvm_ioreq_needs_completion()? I suspect it will remain x86 specific, without any need for an Arm variant.
Paul
> --
> Regards,
>
> Oleksandr Tyshchenko
next prev parent reply other threads:[~2020-08-04 11:23 UTC|newest]
Thread overview: 140+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-03 18:21 [RFC PATCH V1 00/12] IOREQ feature (+ virtio-mmio) on Arm Oleksandr Tyshchenko
2020-08-03 18:21 ` [RFC PATCH V1 01/12] hvm/ioreq: Make x86's IOREQ feature common Oleksandr Tyshchenko
2020-08-04 7:45 ` Paul Durrant
2020-08-04 11:10 ` Oleksandr
2020-08-04 11:23 ` Paul Durrant [this message]
2020-08-04 11:51 ` Oleksandr
2020-08-04 13:18 ` Paul Durrant
2020-08-04 13:52 ` Julien Grall
2020-08-04 15:41 ` Jan Beulich
2020-08-04 19:11 ` Stefano Stabellini
2020-08-05 7:01 ` Jan Beulich
2020-08-06 0:37 ` Stefano Stabellini
2020-08-06 6:59 ` Jan Beulich
2020-08-06 20:32 ` Stefano Stabellini
2020-08-07 13:19 ` Oleksandr
2020-08-07 16:45 ` Oleksandr
2020-08-07 21:50 ` Stefano Stabellini
2020-08-07 22:19 ` Oleksandr
2020-08-10 13:41 ` Oleksandr
2020-08-10 23:34 ` Stefano Stabellini
2020-08-11 9:19 ` Julien Grall
2020-08-11 10:10 ` Oleksandr
2020-08-11 22:47 ` Stefano Stabellini
2020-08-12 14:35 ` Oleksandr
2020-08-12 23:08 ` Stefano Stabellini
2020-08-13 20:16 ` Julien Grall
2020-08-07 23:45 ` Oleksandr
2020-08-10 23:34 ` Stefano Stabellini
2020-08-05 8:33 ` Julien Grall
2020-08-06 0:37 ` Stefano Stabellini
2020-08-06 9:45 ` Julien Grall
2020-08-06 23:48 ` Stefano Stabellini
2020-08-10 19:20 ` Julien Grall
2020-08-10 23:34 ` Stefano Stabellini
2020-08-11 11:28 ` Julien Grall
2020-08-11 22:48 ` Stefano Stabellini
2020-08-12 8:19 ` Julien Grall
2020-08-20 19:14 ` Oleksandr
2020-08-21 0:53 ` Stefano Stabellini
2020-08-21 18:54 ` Julien Grall
2020-08-05 13:30 ` Julien Grall
2020-08-06 11:37 ` Oleksandr
2020-08-10 16:29 ` Julien Grall
2020-08-10 17:28 ` Oleksandr
2020-08-05 16:15 ` Andrew Cooper
2020-08-06 8:20 ` Oleksandr
2020-08-15 17:30 ` Julien Grall
2020-08-16 19:37 ` Oleksandr
2020-08-03 18:21 ` [RFC PATCH V1 02/12] hvm/dm: Make x86's DM " Oleksandr Tyshchenko
2020-08-03 18:21 ` [RFC PATCH V1 03/12] xen/mm: Make x86's XENMEM_resource_ioreq_server handling common Oleksandr Tyshchenko
2020-08-03 18:21 ` [RFC PATCH V1 04/12] xen/arm: Introduce arch specific bits for IOREQ/DM features Oleksandr Tyshchenko
2020-08-04 7:49 ` Paul Durrant
2020-08-04 14:01 ` Julien Grall
2020-08-04 23:22 ` Stefano Stabellini
2020-08-15 17:56 ` Julien Grall
2020-08-17 14:36 ` Oleksandr
2020-08-04 23:22 ` Stefano Stabellini
2020-08-05 7:05 ` Jan Beulich
2020-08-05 16:41 ` Stefano Stabellini
2020-08-05 19:45 ` Oleksandr
2020-08-05 9:32 ` Julien Grall
2020-08-05 15:41 ` Oleksandr
2020-08-06 10:19 ` Julien Grall
2020-08-10 18:09 ` Oleksandr
2020-08-10 18:21 ` Oleksandr
2020-08-10 19:00 ` Julien Grall
2020-08-10 20:29 ` Oleksandr
2020-08-10 22:37 ` Julien Grall
2020-08-11 6:13 ` Oleksandr
2020-08-12 15:08 ` Oleksandr
2020-08-11 17:09 ` Oleksandr
2020-08-11 17:50 ` Julien Grall
2020-08-13 18:41 ` Oleksandr
2020-08-13 20:36 ` Julien Grall
2020-08-13 21:49 ` Oleksandr
2020-08-13 20:39 ` Oleksandr Tyshchenko
2020-08-13 22:14 ` Julien Grall
2020-08-14 12:08 ` Oleksandr
2020-08-05 14:12 ` Julien Grall
2020-08-05 14:45 ` Jan Beulich
2020-08-05 19:30 ` Oleksandr
2020-08-06 11:08 ` Julien Grall
2020-08-06 11:29 ` Jan Beulich
2020-08-20 18:30 ` Oleksandr
2020-08-21 6:16 ` Jan Beulich
2020-08-21 11:13 ` Oleksandr
2020-08-06 13:27 ` Oleksandr
2020-08-10 18:25 ` Julien Grall
2020-08-10 19:58 ` Oleksandr
2020-08-05 16:13 ` Jan Beulich
2020-08-05 19:47 ` Oleksandr
2020-08-03 18:21 ` [RFC PATCH V1 05/12] hvm/dm: Introduce xendevicemodel_set_irq_level DM op Oleksandr Tyshchenko
2020-08-04 23:22 ` Stefano Stabellini
2020-08-05 9:39 ` Julien Grall
2020-08-06 0:37 ` Stefano Stabellini
2020-08-06 11:32 ` Julien Grall
2020-08-06 23:49 ` Stefano Stabellini
2020-08-07 8:43 ` Jan Beulich
2020-08-07 21:50 ` Stefano Stabellini
2020-08-08 9:27 ` Julien Grall
2020-08-08 9:28 ` Julien Grall
2020-08-10 23:34 ` Stefano Stabellini
2020-08-11 13:04 ` Julien Grall
2020-08-11 22:48 ` Stefano Stabellini
2020-08-18 9:31 ` Julien Grall
2020-08-21 0:53 ` Stefano Stabellini
2020-08-17 15:23 ` Jan Beulich
2020-08-17 22:56 ` Stefano Stabellini
2020-08-18 8:03 ` Jan Beulich
2020-08-05 16:15 ` Jan Beulich
2020-08-05 22:12 ` Oleksandr
2020-08-03 18:21 ` [RFC PATCH V1 06/12] libxl: Introduce basic virtio-mmio support on Arm Oleksandr Tyshchenko
2020-08-03 18:21 ` [RFC PATCH V1 07/12] A collection of tweaks to be able to run emulator in driver domain Oleksandr Tyshchenko
2020-08-05 16:19 ` Jan Beulich
2020-08-05 16:40 ` Paul Durrant
2020-08-06 9:22 ` Oleksandr
2020-08-06 9:27 ` Jan Beulich
2020-08-14 16:30 ` Oleksandr
2020-08-16 15:36 ` Julien Grall
2020-08-17 15:07 ` Oleksandr
2020-08-03 18:21 ` [RFC PATCH V1 08/12] xen/arm: Invalidate qemu mapcache on XENMEM_decrease_reservation Oleksandr Tyshchenko
2020-08-05 16:21 ` Jan Beulich
2020-08-06 11:35 ` Julien Grall
2020-08-06 11:50 ` Jan Beulich
2020-08-06 14:28 ` Oleksandr
2020-08-06 16:33 ` Jan Beulich
2020-08-06 16:57 ` Oleksandr
2020-08-03 18:21 ` [RFC PATCH V1 09/12] libxl: Handle virtio-mmio irq in more correct way Oleksandr Tyshchenko
2020-08-04 23:22 ` Stefano Stabellini
2020-08-05 20:51 ` Oleksandr
2020-08-03 18:21 ` [RFC PATCH V1 10/12] libxl: Add support for virtio-disk configuration Oleksandr Tyshchenko
2020-08-04 23:23 ` Stefano Stabellini
2020-08-05 21:12 ` Oleksandr
2020-08-06 0:37 ` Stefano Stabellini
2020-08-03 18:21 ` [RFC PATCH V1 11/12] libxl: Insert "dma-coherent" property into virtio-mmio device node Oleksandr Tyshchenko
2020-08-04 23:23 ` Stefano Stabellini
2020-08-05 20:35 ` Oleksandr
2020-08-03 18:21 ` [RFC PATCH V1 12/12] libxl: Fix duplicate memory node in DT Oleksandr Tyshchenko
2020-08-15 17:24 ` [RFC PATCH V1 00/12] IOREQ feature (+ virtio-mmio) on Arm Julien Grall
2020-08-16 19:34 ` Oleksandr
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='002101d66a51$b4c59070$1e50b150$@xen.org' \
--to=xadimgnik@gmail.com \
--cc=andrew.cooper3@citrix.com \
--cc=george.dunlap@citrix.com \
--cc=ian.jackson@eu.citrix.com \
--cc=jbeulich@suse.com \
--cc=julien.grall@arm.com \
--cc=julien@xen.org \
--cc=jun.nakajima@intel.com \
--cc=kevin.tian@intel.com \
--cc=oleksandr_tyshchenko@epam.com \
--cc=olekstysh@gmail.com \
--cc=paul@xen.org \
--cc=roger.pau@citrix.com \
--cc=sstabellini@kernel.org \
--cc=tim@xen.org \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).