Mailing List Archive

[RFC PATCH V1 08/12] xen/arm: Invalidate qemu mapcache on XENMEM_decrease_reservation
From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

Borrow x86's logic to invalidate qemu mapcache.

TODO: Move send_invalidate_req() to common code (ioreq.c?).

Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
---
xen/arch/arm/ioreq.c | 14 ++++++++++++++
xen/arch/arm/traps.c | 6 ++++++
xen/common/memory.c | 6 ++++++
xen/include/asm-arm/domain.h | 2 ++
xen/include/asm-arm/hvm/ioreq.h | 2 ++
5 files changed, 30 insertions(+)

diff --git a/xen/arch/arm/ioreq.c b/xen/arch/arm/ioreq.c
index a9cc839..8f60c41 100644
--- a/xen/arch/arm/ioreq.c
+++ b/xen/arch/arm/ioreq.c
@@ -75,6 +75,20 @@ bool handle_mmio(void)
return true;
}

+/* Ask ioemu mapcache to invalidate mappings. */
+void send_invalidate_req(void)
+{
+ ioreq_t p = {
+ .type = IOREQ_TYPE_INVALIDATE,
+ .size = 4,
+ .dir = IOREQ_WRITE,
+ .data = ~0UL, /* flush all */
+ };
+
+ if ( hvm_broadcast_ioreq(&p, false) != 0 )
+ gprintk(XENLOG_ERR, "Unsuccessful map-cache invalidate\n");
+}
+
/*
* Local variables:
* mode: C
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 4cdf098..ea472d1 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1490,6 +1490,12 @@ static void do_trap_hypercall(struct cpu_user_regs *regs, register_t *nr,
/* Ensure the hypercall trap instruction is re-executed. */
if ( current->hcall_preempted )
regs->pc -= 4; /* re-execute 'hvc #XEN_HYPERCALL_TAG' */
+
+#ifdef CONFIG_IOREQ_SERVER
+ if ( unlikely(current->domain->arch.hvm.qemu_mapcache_invalidate) &&
+ test_and_clear_bool(current->domain->arch.hvm.qemu_mapcache_invalidate) )
+ send_invalidate_req();
+#endif
}

void arch_hypercall_tasklet_result(struct vcpu *v, long res)
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 8b306f6..8d9f0a8 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -1652,6 +1652,12 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
break;
}

+ /* x86 already sets the flag in hvm_memory_op() */
+#if defined(CONFIG_ARM64) && defined(CONFIG_IOREQ_SERVER)
+ if ( op == XENMEM_decrease_reservation )
+ curr_d->arch.hvm.qemu_mapcache_invalidate = true;
+#endif
+
return rc;
}

diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index e060b0a..0db8bb4 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -69,6 +69,8 @@ struct hvm_domain
spinlock_t lock;
struct hvm_ioreq_server *server[MAX_NR_IOREQ_SERVERS];
} ioreq_server;
+
+ bool_t qemu_mapcache_invalidate;
};

#ifdef CONFIG_ARM_64
diff --git a/xen/include/asm-arm/hvm/ioreq.h b/xen/include/asm-arm/hvm/ioreq.h
index 83a560c..392ce64 100644
--- a/xen/include/asm-arm/hvm/ioreq.h
+++ b/xen/include/asm-arm/hvm/ioreq.h
@@ -90,6 +90,8 @@ static inline void arch_hvm_ioreq_destroy(struct domain *d)
#define IOREQ_IO_UNHANDLED IO_UNHANDLED
#define IOREQ_IO_RETRY IO_RETRY

+void send_invalidate_req(void);
+
#endif /* __ASM_X86_HVM_IOREQ_H__ */

/*
--
2.7.4
Re: [RFC PATCH V1 08/12] xen/arm: Invalidate qemu mapcache on XENMEM_decrease_reservation [ In reply to ]
On 03.08.2020 20:21, Oleksandr Tyshchenko wrote:
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -1652,6 +1652,12 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> break;
> }
>
> + /* x86 already sets the flag in hvm_memory_op() */
> +#if defined(CONFIG_ARM64) && defined(CONFIG_IOREQ_SERVER)
> + if ( op == XENMEM_decrease_reservation )
> + curr_d->arch.hvm.qemu_mapcache_invalidate = true;
> +#endif

Doesn't the comment already indicate a route towards an approach
not requiring to alter common code?

Jan
Re: [RFC PATCH V1 08/12] xen/arm: Invalidate qemu mapcache on XENMEM_decrease_reservation [ In reply to ]
Hi Jan,

On 05/08/2020 17:21, Jan Beulich wrote:
> On 03.08.2020 20:21, Oleksandr Tyshchenko wrote:
>> --- a/xen/common/memory.c
>> +++ b/xen/common/memory.c
>> @@ -1652,6 +1652,12 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>> break;
>> }
>>
>> + /* x86 already sets the flag in hvm_memory_op() */
>> +#if defined(CONFIG_ARM64) && defined(CONFIG_IOREQ_SERVER)
>> + if ( op == XENMEM_decrease_reservation )
>> + curr_d->arch.hvm.qemu_mapcache_invalidate = true;
>> +#endif
>
> Doesn't the comment already indicate a route towards an approach
> not requiring to alter common code?

Given that IOREQ is now moved under common/, I think it would make sense
to have this set in common code as well for all the architecture.

IOW, I would suggest to drop the #ifdef CONFIG_ARM64. In addition, we
may want to introduce an helper to check if a domain is using ioreq.

Cheers,

--
Julien Grall
Re: [RFC PATCH V1 08/12] xen/arm: Invalidate qemu mapcache on XENMEM_decrease_reservation [ In reply to ]
On 06.08.2020 13:35, Julien Grall wrote:
> On 05/08/2020 17:21, Jan Beulich wrote:
>> On 03.08.2020 20:21, Oleksandr Tyshchenko wrote:
>>> --- a/xen/common/memory.c
>>> +++ b/xen/common/memory.c
>>> @@ -1652,6 +1652,12 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>>> break;
>>> }
>>>
>>> + /* x86 already sets the flag in hvm_memory_op() */
>>> +#if defined(CONFIG_ARM64) && defined(CONFIG_IOREQ_SERVER)
>>> + if ( op == XENMEM_decrease_reservation )
>>> + curr_d->arch.hvm.qemu_mapcache_invalidate = true;
>>> +#endif
>>
>> Doesn't the comment already indicate a route towards an approach
>> not requiring to alter common code?
>
> Given that IOREQ is now moved under common/, I think it would make sense
> to have this set in common code as well for all the architecture.
>
> IOW, I would suggest to drop the #ifdef CONFIG_ARM64. In addition, we
> may want to introduce an helper to check if a domain is using ioreq.

Of course, with the (part of the) conditional dropped and the struct
field moved out of the arch sub-struct, this is fine to live here.

Jan
Re: [RFC PATCH V1 08/12] xen/arm: Invalidate qemu mapcache on XENMEM_decrease_reservation [ In reply to ]
On 06.08.20 14:50, Jan Beulich wrote:

Hi Jan

>>> On 03.08.2020 20:21, Oleksandr Tyshchenko wrote:
>>>> --- a/xen/common/memory.c
>>>> +++ b/xen/common/memory.c
>>>> @@ -1652,6 +1652,12 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>>>> break;
>>>> }
>>>>
>>>> + /* x86 already sets the flag in hvm_memory_op() */
>>>> +#if defined(CONFIG_ARM64) && defined(CONFIG_IOREQ_SERVER)
>>>> + if ( op == XENMEM_decrease_reservation )
>>>> + curr_d->arch.hvm.qemu_mapcache_invalidate = true;
>>>> +#endif
>>> Doesn't the comment already indicate a route towards an approach
>>> not requiring to alter common code?
>> Given that IOREQ is now moved under common/, I think it would make sense
>> to have this set in common code as well for all the architecture.
>>
>> IOW, I would suggest to drop the #ifdef CONFIG_ARM64. In addition, we
>> may want to introduce an helper to check if a domain is using ioreq.
> Of course, with the (part of the) conditional dropped and the struct
> field moved out of the arch sub-struct, this is fine to live here.

ok.


I suspect this should *also* live in compat_memory_op(). Please confirm
whether my understanding correct.


--
Regards,

Oleksandr Tyshchenko
Re: [RFC PATCH V1 08/12] xen/arm: Invalidate qemu mapcache on XENMEM_decrease_reservation [ In reply to ]
On 06.08.2020 16:28, Oleksandr wrote:
>
> On 06.08.20 14:50, Jan Beulich wrote:
>
> Hi Jan
>
>>>> On 03.08.2020 20:21, Oleksandr Tyshchenko wrote:
>>>>> --- a/xen/common/memory.c
>>>>> +++ b/xen/common/memory.c
>>>>> @@ -1652,6 +1652,12 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>>>>> break;
>>>>> }
>>>>>
>>>>> + /* x86 already sets the flag in hvm_memory_op() */
>>>>> +#if defined(CONFIG_ARM64) && defined(CONFIG_IOREQ_SERVER)
>>>>> + if ( op == XENMEM_decrease_reservation )
>>>>> + curr_d->arch.hvm.qemu_mapcache_invalidate = true;
>>>>> +#endif
>>>> Doesn't the comment already indicate a route towards an approach
>>>> not requiring to alter common code?
>>> Given that IOREQ is now moved under common/, I think it would make sense
>>> to have this set in common code as well for all the architecture.
>>>
>>> IOW, I would suggest to drop the #ifdef CONFIG_ARM64. In addition, we
>>> may want to introduce an helper to check if a domain is using ioreq.
>> Of course, with the (part of the) conditional dropped and the struct
>> field moved out of the arch sub-struct, this is fine to live here.
>
> ok.
>
>
> I suspect this should *also* live in compat_memory_op(). Please confirm
> whether my understanding correct.

Doesn't compat_memory_op() simply call here, so will have the flag set
as needed?

Jan
Re: [RFC PATCH V1 08/12] xen/arm: Invalidate qemu mapcache on XENMEM_decrease_reservation [ In reply to ]
On 06.08.20 19:33, Jan Beulich wrote:

Hi Jan.

> On 06.08.2020 16:28, Oleksandr wrote:
>> On 06.08.20 14:50, Jan Beulich wrote:
>>
>> Hi Jan
>>
>>>>> On 03.08.2020 20:21, Oleksandr Tyshchenko wrote:
>>>>>> --- a/xen/common/memory.c
>>>>>> +++ b/xen/common/memory.c
>>>>>> @@ -1652,6 +1652,12 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>>>>>> break;
>>>>>> }
>>>>>>
>>>>>> + /* x86 already sets the flag in hvm_memory_op() */
>>>>>> +#if defined(CONFIG_ARM64) && defined(CONFIG_IOREQ_SERVER)
>>>>>> + if ( op == XENMEM_decrease_reservation )
>>>>>> + curr_d->arch.hvm.qemu_mapcache_invalidate = true;
>>>>>> +#endif
>>>>> Doesn't the comment already indicate a route towards an approach
>>>>> not requiring to alter common code?
>>>> Given that IOREQ is now moved under common/, I think it would make sense
>>>> to have this set in common code as well for all the architecture.
>>>>
>>>> IOW, I would suggest to drop the #ifdef CONFIG_ARM64. In addition, we
>>>> may want to introduce an helper to check if a domain is using ioreq.
>>> Of course, with the (part of the) conditional dropped and the struct
>>> field moved out of the arch sub-struct, this is fine to live here.
>> ok.
>>
>>
>> I suspect this should *also* live in compat_memory_op(). Please confirm
>> whether my understanding correct.
> Doesn't compat_memory_op() simply call here, so will have the flag set
> as needed?
Indeed, sorry for the noise.


--
Regards,

Oleksandr Tyshchenko