From: Shuo Liu <[email protected]>
The ACRN Hypervisor builds an I/O request when a trapped I/O access
happens in User VM. Then, ACRN Hypervisor issues an upcall by sending
a notification interrupt to the Service VM. HSM in the Service VM needs
to hook the notification interrupt to handle I/O requests.
Notification interrupts from ACRN Hypervisor are already supported and
a, currently uninitialized, callback called.
Export two APIs for HSM to setup/remove its callback.
Originally-by: Yakui Zhao <[email protected]>
Signed-off-by: Shuo Liu <[email protected]>
Reviewed-by: Zhi Wang <[email protected]>
Reviewed-by: Reinette Chatre <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: Sean Christopherson <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: Fengwei Yin <[email protected]>
Cc: Zhi Wang <[email protected]>
Cc: Zhenyu Wang <[email protected]>
Cc: Yu Wang <[email protected]>
Cc: Reinette Chatre <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
---
arch/x86/include/asm/acrn.h | 8 ++++++++
arch/x86/kernel/cpu/acrn.c | 16 ++++++++++++++++
2 files changed, 24 insertions(+)
create mode 100644 arch/x86/include/asm/acrn.h
diff --git a/arch/x86/include/asm/acrn.h b/arch/x86/include/asm/acrn.h
new file mode 100644
index 000000000000..ff259b69cde7
--- /dev/null
+++ b/arch/x86/include/asm/acrn.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_ACRN_H
+#define _ASM_X86_ACRN_H
+
+void acrn_setup_intr_handler(void (*handler)(void));
+void acrn_remove_intr_handler(void);
+
+#endif /* _ASM_X86_ACRN_H */
diff --git a/arch/x86/kernel/cpu/acrn.c b/arch/x86/kernel/cpu/acrn.c
index 0b2c03943ac6..42e88d01ccf9 100644
--- a/arch/x86/kernel/cpu/acrn.c
+++ b/arch/x86/kernel/cpu/acrn.c
@@ -9,7 +9,11 @@
*
*/
+#define pr_fmt(fmt) "acrn: " fmt
+
#include <linux/interrupt.h>
+
+#include <asm/acrn.h>
#include <asm/apic.h>
#include <asm/cpufeatures.h>
#include <asm/desc.h>
@@ -55,6 +59,18 @@ DEFINE_IDTENTRY_SYSVEC(sysvec_acrn_hv_callback)
set_irq_regs(old_regs);
}
+void acrn_setup_intr_handler(void (*handler)(void))
+{
+ acrn_intr_handler = handler;
+}
+EXPORT_SYMBOL_GPL(acrn_setup_intr_handler);
+
+void acrn_remove_intr_handler(void)
+{
+ acrn_intr_handler = NULL;
+}
+EXPORT_SYMBOL_GPL(acrn_remove_intr_handler);
+
const __initconst struct hypervisor_x86 x86_hyper_acrn = {
.name = "ACRN",
.detect = acrn_detect,
--
2.28.0
On Tue, Sep 22, 2020 at 07:42:56PM +0800, [email protected] wrote:
> From: Shuo Liu <[email protected]>
>
> The ACRN Hypervisor builds an I/O request when a trapped I/O access
> happens in User VM. Then, ACRN Hypervisor issues an upcall by sending
> a notification interrupt to the Service VM. HSM in the Service VM needs
> to hook the notification interrupt to handle I/O requests.
>
> Notification interrupts from ACRN Hypervisor are already supported and
> a, currently uninitialized, callback called.
>
> Export two APIs for HSM to setup/remove its callback.
>
> Originally-by: Yakui Zhao <[email protected]>
> Signed-off-by: Shuo Liu <[email protected]>
> Reviewed-by: Zhi Wang <[email protected]>
> Reviewed-by: Reinette Chatre <[email protected]>
> Cc: Dave Hansen <[email protected]>
> Cc: Sean Christopherson <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: Fengwei Yin <[email protected]>
> Cc: Zhi Wang <[email protected]>
> Cc: Zhenyu Wang <[email protected]>
> Cc: Yu Wang <[email protected]>
> Cc: Reinette Chatre <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> ---
> arch/x86/include/asm/acrn.h | 8 ++++++++
> arch/x86/kernel/cpu/acrn.c | 16 ++++++++++++++++
> 2 files changed, 24 insertions(+)
> create mode 100644 arch/x86/include/asm/acrn.h
>
> diff --git a/arch/x86/include/asm/acrn.h b/arch/x86/include/asm/acrn.h
> new file mode 100644
> index 000000000000..ff259b69cde7
> --- /dev/null
> +++ b/arch/x86/include/asm/acrn.h
> @@ -0,0 +1,8 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_X86_ACRN_H
> +#define _ASM_X86_ACRN_H
> +
> +void acrn_setup_intr_handler(void (*handler)(void));
> +void acrn_remove_intr_handler(void);
> +
> +#endif /* _ASM_X86_ACRN_H */
> diff --git a/arch/x86/kernel/cpu/acrn.c b/arch/x86/kernel/cpu/acrn.c
> index 0b2c03943ac6..42e88d01ccf9 100644
> --- a/arch/x86/kernel/cpu/acrn.c
> +++ b/arch/x86/kernel/cpu/acrn.c
> @@ -9,7 +9,11 @@
> *
> */
>
> +#define pr_fmt(fmt) "acrn: " fmt
Why is this needed, if you are not adding pr_* calls in this patch?
thanks,
greg k-h
On Sun 27.Sep'20 at 12:49:43 +0200, Greg Kroah-Hartman wrote:
>On Tue, Sep 22, 2020 at 07:42:56PM +0800, [email protected] wrote:
>> From: Shuo Liu <[email protected]>
>>
>> The ACRN Hypervisor builds an I/O request when a trapped I/O access
>> happens in User VM. Then, ACRN Hypervisor issues an upcall by sending
>> a notification interrupt to the Service VM. HSM in the Service VM needs
>> to hook the notification interrupt to handle I/O requests.
>>
>> Notification interrupts from ACRN Hypervisor are already supported and
>> a, currently uninitialized, callback called.
>>
>> Export two APIs for HSM to setup/remove its callback.
>>
>> Originally-by: Yakui Zhao <[email protected]>
>> Signed-off-by: Shuo Liu <[email protected]>
>> Reviewed-by: Zhi Wang <[email protected]>
>> Reviewed-by: Reinette Chatre <[email protected]>
>> Cc: Dave Hansen <[email protected]>
>> Cc: Sean Christopherson <[email protected]>
>> Cc: Dan Williams <[email protected]>
>> Cc: Fengwei Yin <[email protected]>
>> Cc: Zhi Wang <[email protected]>
>> Cc: Zhenyu Wang <[email protected]>
>> Cc: Yu Wang <[email protected]>
>> Cc: Reinette Chatre <[email protected]>
>> Cc: Greg Kroah-Hartman <[email protected]>
>> ---
>> arch/x86/include/asm/acrn.h | 8 ++++++++
>> arch/x86/kernel/cpu/acrn.c | 16 ++++++++++++++++
>> 2 files changed, 24 insertions(+)
>> create mode 100644 arch/x86/include/asm/acrn.h
>>
>> diff --git a/arch/x86/include/asm/acrn.h b/arch/x86/include/asm/acrn.h
>> new file mode 100644
>> index 000000000000..ff259b69cde7
>> --- /dev/null
>> +++ b/arch/x86/include/asm/acrn.h
>> @@ -0,0 +1,8 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef _ASM_X86_ACRN_H
>> +#define _ASM_X86_ACRN_H
>> +
>> +void acrn_setup_intr_handler(void (*handler)(void));
>> +void acrn_remove_intr_handler(void);
>> +
>> +#endif /* _ASM_X86_ACRN_H */
>> diff --git a/arch/x86/kernel/cpu/acrn.c b/arch/x86/kernel/cpu/acrn.c
>> index 0b2c03943ac6..42e88d01ccf9 100644
>> --- a/arch/x86/kernel/cpu/acrn.c
>> +++ b/arch/x86/kernel/cpu/acrn.c
>> @@ -9,7 +9,11 @@
>> *
>> */
>>
>> +#define pr_fmt(fmt) "acrn: " fmt
>
>Why is this needed, if you are not adding pr_* calls in this patch?
True. I will remove it. Thanks.
On Tue, Sep 22, 2020 at 07:42:56PM +0800, [email protected] wrote:
> +void acrn_setup_intr_handler(void (*handler)(void))
> +{
> + acrn_intr_handler = handler;
> +}
> +EXPORT_SYMBOL_GPL(acrn_setup_intr_handler);
> +
> +void acrn_remove_intr_handler(void)
> +{
> + acrn_intr_handler = NULL;
> +}
> +EXPORT_SYMBOL_GPL(acrn_remove_intr_handler);
I don't like this one bit.
Also, what stops the module from doing acrn_remove_intr_handler()
while it gets a HYPERVISOR_CALLBACK_VECTOR interrupt and the handler
disappearing from under it?
IOW, this should be an atomic notifier instead.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Tue, Sep 29 2020 at 20:01, Borislav Petkov wrote:
> On Tue, Sep 22, 2020 at 07:42:56PM +0800, [email protected] wrote:
>> +void acrn_setup_intr_handler(void (*handler)(void))
>> +{
>> + acrn_intr_handler = handler;
>> +}
>> +EXPORT_SYMBOL_GPL(acrn_setup_intr_handler);
>> +
>> +void acrn_remove_intr_handler(void)
>> +{
>> + acrn_intr_handler = NULL;
>> +}
>> +EXPORT_SYMBOL_GPL(acrn_remove_intr_handler);
>
> I don't like this one bit.
>
> Also, what stops the module from doing acrn_remove_intr_handler()
> while it gets a HYPERVISOR_CALLBACK_VECTOR interrupt and the handler
> disappearing from under it?
>
> IOW, this should be an atomic notifier instead.
That does not prevent that either and notifiers suck. The pointer is
fine and if something removes the handler before all of the muck is
shutdown then the author can keep the pieces and mop up the remains.
Thanks,
tglx
On Tue, Sep 29, 2020 at 10:07:17PM +0200, Thomas Gleixner wrote:
> That does not prevent that either and notifiers suck.
Bah, atomic notifiers run functions which cannot block, not what is
needed here, right.
> The pointer is fine and if something removes the handler before all of
> the muck is shutdown then the author can keep the pieces and mop up
> the remains.
Uhu, so what makes sure that the module is not removed while an IRQ is
happening?
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Hi Boris,
On Tue 29.Sep'20 at 22:26:54 +0200, Borislav Petkov wrote:
>On Tue, Sep 29, 2020 at 10:07:17PM +0200, Thomas Gleixner wrote:
>> That does not prevent that either and notifiers suck.
>
>Bah, atomic notifiers run functions which cannot block, not what is
>needed here, right.
>
>> The pointer is fine and if something removes the handler before all of
>> the muck is shutdown then the author can keep the pieces and mop up
>> the remains.
>
>Uhu, so what makes sure that the module is not removed while an IRQ is
>happening?
The precondition of the removing of the module is that there is no
User VM living (every opening of the dev file will hold a ref count of
the module). The interrupt only can occur with active User VMs. So if
a notification interrupt is happending, the module cannot be removed as
there is still User VM living.
Thanks
shuo