diff mbox series

[V3,03/23] x86/ioreq: Provide out-of-line wrapper for the handle_mmio()

Message ID 1606732298-22107-4-git-send-email-olekstysh@gmail.com (mailing list archive)
State New, archived
Headers show
Series [V3,01/23] x86/ioreq: Prepare IOREQ feature for making it common | expand

Commit Message

Oleksandr Nov. 30, 2020, 10:31 a.m. UTC
From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

The IOREQ is about to be common feature and Arm will have its own
implementation.

But the name of the function is pretty generic and can be confusing
on Arm (we already have a try_handle_mmio()).

In order not to rename the function (which is used for a varying
set of purposes on x86) globally and get non-confusing variant on Arm
provide a wrapper ioreq_complete_mmio() to be used on common and Arm code.

Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
CC: Julien Grall <julien.grall@arm.com>

---
Please note, this is a split/cleanup/hardening of Julien's PoC:
"Add support for Guest IO forwarding to a device emulator"

Changes RFC -> V1:
   - new patch

Changes V1 -> V2:
   - remove "handle"
   - add Jan's A-b

Changes V2 -> V3:
   - remove Jan's A-b
   - update patch subject/description
   - use out-of-line function instead of #define
   - put earlier in the series to avoid breakage
---
---
 xen/arch/x86/hvm/ioreq.c        | 7 ++++++-
 xen/include/asm-x86/hvm/ioreq.h | 2 ++
 2 files changed, 8 insertions(+), 1 deletion(-)

Comments

Jan Beulich Dec. 7, 2020, 11:27 a.m. UTC | #1
On 30.11.2020 11:31, Oleksandr Tyshchenko wrote:
> --- a/xen/arch/x86/hvm/ioreq.c
> +++ b/xen/arch/x86/hvm/ioreq.c
> @@ -36,6 +36,11 @@
>  #include <public/hvm/ioreq.h>
>  #include <public/hvm/params.h>
>  
> +bool ioreq_complete_mmio(void)
> +{
> +    return handle_mmio();
> +}

As indicated before I don't like out-of-line functions like this
one; I think a #define would be quite fine here, but Paul as the
maintainer thinks differently. So be it. However, shouldn't this
function be named arch_ioreq_complete_mmio() according to the
new naming model, and then ...

> --- a/xen/include/asm-x86/hvm/ioreq.h
> +++ b/xen/include/asm-x86/hvm/ioreq.h
> @@ -74,6 +74,8 @@ unsigned int hvm_broadcast_ioreq(ioreq_t *p, bool buffered);
>  
>  void hvm_ioreq_init(struct domain *d);
>  
> +bool ioreq_complete_mmio(void);

... get declared next to the other arch_*() hooks? With this
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan
Oleksandr Dec. 7, 2020, 3:39 p.m. UTC | #2
On 07.12.20 13:27, Jan Beulich wrote:

Hi Jan

> On 30.11.2020 11:31, Oleksandr Tyshchenko wrote:
>> --- a/xen/arch/x86/hvm/ioreq.c
>> +++ b/xen/arch/x86/hvm/ioreq.c
>> @@ -36,6 +36,11 @@
>>   #include <public/hvm/ioreq.h>
>>   #include <public/hvm/params.h>
>>   
>> +bool ioreq_complete_mmio(void)
>> +{
>> +    return handle_mmio();
>> +}
> As indicated before I don't like out-of-line functions like this
> one; I think a #define would be quite fine here, but Paul as the
> maintainer thinks differently. So be it. However, shouldn't this
> function be named arch_ioreq_complete_mmio() according to the
> new naming model, and then ...
>
>> --- a/xen/include/asm-x86/hvm/ioreq.h
>> +++ b/xen/include/asm-x86/hvm/ioreq.h
>> @@ -74,6 +74,8 @@ unsigned int hvm_broadcast_ioreq(ioreq_t *p, bool buffered);
>>   
>>   void hvm_ioreq_init(struct domain *d);
>>   
>> +bool ioreq_complete_mmio(void);
> ... get declared next to the other arch_*() hooks? With this

sounds reasonable, will update


> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Thank you
diff mbox series

Patch

diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
index 9525554..36b1e4e 100644
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -36,6 +36,11 @@ 
 #include <public/hvm/ioreq.h>
 #include <public/hvm/params.h>
 
+bool ioreq_complete_mmio(void)
+{
+    return handle_mmio();
+}
+
 static void set_ioreq_server(struct domain *d, unsigned int id,
                              struct hvm_ioreq_server *s)
 {
@@ -226,7 +231,7 @@  bool handle_hvm_io_completion(struct vcpu *v)
         break;
 
     case HVMIO_mmio_completion:
-        return handle_mmio();
+        return ioreq_complete_mmio();
 
     case HVMIO_pio_completion:
         return handle_pio(vio->io_req.addr, vio->io_req.size,
diff --git a/xen/include/asm-x86/hvm/ioreq.h b/xen/include/asm-x86/hvm/ioreq.h
index e9c8b2d..c7563e1 100644
--- a/xen/include/asm-x86/hvm/ioreq.h
+++ b/xen/include/asm-x86/hvm/ioreq.h
@@ -74,6 +74,8 @@  unsigned int hvm_broadcast_ioreq(ioreq_t *p, bool buffered);
 
 void hvm_ioreq_init(struct domain *d);
 
+bool ioreq_complete_mmio(void);
+
 #define IOREQ_STATUS_HANDLED     X86EMUL_OKAY
 #define IOREQ_STATUS_UNHANDLED   X86EMUL_UNHANDLEABLE
 #define IOREQ_STATUS_RETRY       X86EMUL_RETRY