[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH V1 09/16] arm/ioreq: Introduce arch specific bits for IOREQ/DM features



Hi,

On 24/09/2020 19:02, Oleksandr wrote:
On 24.09.20 19:02, Oleksandr wrote:
On 24.09.20 14:08, Jan Beulich wrote:
On 23.09.2020 22:16, Oleksandr wrote:
On 23.09.20 21:03, Julien Grall wrote:
On 10/09/2020 21:22, Oleksandr Tyshchenko wrote:
From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
@@ -91,6 +108,28 @@ struct arch_domain
   #endif
   }  __cacheline_aligned;
   +enum hvm_io_completion {
+    HVMIO_no_completion,
+    HVMIO_mmio_completion,
+    HVMIO_pio_completion
+};
+
+struct hvm_vcpu_io {
+    /* I/O request in flight to device model. */
+    enum hvm_io_completion io_completion;
+    ioreq_t                io_req;
+
+    /*
+     * HVM emulation:
+     *  Linear address @mmio_gla maps to MMIO physical frame
@mmio_gpfn.
+     *  The latter is known to be an MMIO frame (not RAM).
+     *  This translation is only valid for accesses as per
@mmio_access.
+     */
+    struct npfec        mmio_access;
+    unsigned long       mmio_gla;
+    unsigned long       mmio_gpfn;
+};
+
Why do we need to re-define most of this? Can't this just be in common
code?
Jan asked almost the similar question in "[PATCH V1 02/16] xen/ioreq:
Make x86's IOREQ feature common".
Please see my answer there:
https://patchwork.kernel.org/patch/11769105/#23637511

Theoretically we could move this to the common code, but the question is
how to be with other struct fields the x86's struct hvm_vcpu_io
has/needs but
Arm's seems not, would it be possible to logically split struct
hvm_vcpu_io into common and arch parts?
Have struct vcpu_io and struct arch_vcpu_io as a sub-part of it?
Although it is going to pull a lot of changes into x86/hvm/*, yes this way we indeed could logically split struct hvm_vcpu_io into common and arch parts in a clear way.
If it is really worth it, I will start looking into it.
Julien, I noticed that three fields mmio* are not used within current series on Arm. Do we expect them to be used later on?

IIRC, I just copied them blindly when writing the PoC.

The information can already be found using the HSR (syndrome register), so those fields would be redundant for us.

Would be the following acceptable?
1. Both fields "io_completion" and "io_req" (which seems to be the only fields used in common/ioreq.c) are moved to common struct vcpu as part of struct vcpu_io,
     enum hvm_io_completion is also moved (and renamed).
2. We remove everything related to hvm_vcpu* from the Arm header.
3. x86's struct hvm_vcpu_io stays as it is (but without two fields "io_completion" and "io_req").     I think, this way we separate a common part and reduce Xen changes (which are getting bigger).

The plan looks reasonable to me.

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.