From: Vitaly Kuznetsov <vkuznets@redhat.com>
To: Jinank Jain <jinankjain@linux.microsoft.com>
Cc: kys@microsoft.com, haiyangz@microsoft.com,
sthemmin@microsoft.com, wei.liu@kernel.org, decui@microsoft.com,
tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com,
arnd@arndb.de, peterz@infradead.org, jpoimboe@kernel.org,
jinankjain@linux.microsoft.com, seanjc@google.com,
kirill.shutemov@linux.intel.com, ak@linux.intel.com,
sathyanarayanan.kuppuswamy@linux.intel.com,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arch@vger.kernel.org
Subject: Re: [PATCH 2/6] hv: Setup synic registers in case of nested root partition
Date: Wed, 02 Nov 2022 15:58:48 +0100 [thread overview]
Message-ID: <871qqls8rb.fsf@redhat.com> (raw)
In-Reply-To: <78973f0cfed8a19fced95875c0142a08386e66ed.1667394408.git.jinankjain@microsoft.com>
Jinank Jain <jinankjain@linux.microsoft.com> writes:
> Child partitions are free to allocate SynIC message and event page but in
> case of root partition it must use the pages allocated by Microsoft
> Hypervisor (MSHV). Base address for these pages can be found using
> synthetic MSRs exposed by MSHV. There is a slight difference in those MSRs
> for nested vs non-nested root partition.
>
> Signed-off-by: Jinank Jain <jinankjain@linux.microsoft.com>
> ---
> arch/x86/include/asm/hyperv-tlfs.h | 11 ++++++
> drivers/hv/hv.c | 55 ++++++++++++++++++------------
> 2 files changed, 45 insertions(+), 21 deletions(-)
>
> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
> index d9a611565859..0319091e2019 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -225,6 +225,17 @@ enum hv_isolation_type {
> #define HV_REGISTER_SINT14 0x4000009E
> #define HV_REGISTER_SINT15 0x4000009F
>
> +/*
> + * Define synthetic interrupt controller model specific registers for
> + * nested hypervisor.
> + */
> +#define HV_REGISTER_NESTED_SCONTROL 0x40001080
> +#define HV_REGISTER_NESTED_SVERSION 0x40001081
> +#define HV_REGISTER_NESTED_SIEFP 0x40001082
> +#define HV_REGISTER_NESTED_SIMP 0x40001083
> +#define HV_REGISTER_NESTED_EOM 0x40001084
> +#define HV_REGISTER_NESTED_SINT0 0x40001090
> +
> /*
> * Synthetic Timer MSRs. Four timers per vcpu.
> */
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index 4d6480d57546..92ee910561c4 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -25,6 +25,11 @@
> /* The one and only */
> struct hv_context hv_context;
>
> +#define REG_SIMP (hv_nested ? HV_REGISTER_NESTED_SIMP : HV_REGISTER_SIMP)
> +#define REG_SIEFP (hv_nested ? HV_REGISTER_NESTED_SIEFP : HV_REGISTER_SIEFP)
> +#define REG_SCTRL (hv_nested ? HV_REGISTER_NESTED_SCONTROL : HV_REGISTER_SCONTROL)
> +#define REG_SINT0 (hv_nested ? HV_REGISTER_NESTED_SINT0 : HV_REGISTER_SINT0)
> +
> /*
> * hv_init - Main initialization routine.
> *
> @@ -147,7 +152,7 @@ int hv_synic_alloc(void)
> * Synic message and event pages are allocated by paravisor.
> * Skip these pages allocation here.
> */
> - if (!hv_isolation_type_snp()) {
> + if (!hv_isolation_type_snp() && !hv_root_partition) {
> hv_cpu->synic_message_page =
> (void *)get_zeroed_page(GFP_ATOMIC);
> if (hv_cpu->synic_message_page == NULL) {
> @@ -188,8 +193,16 @@ void hv_synic_free(void)
> struct hv_per_cpu_context *hv_cpu
> = per_cpu_ptr(hv_context.cpu_context, cpu);
>
> - free_page((unsigned long)hv_cpu->synic_event_page);
> - free_page((unsigned long)hv_cpu->synic_message_page);
> + if (hv_root_partition) {
> + if (hv_cpu->synic_event_page != NULL)
> + memunmap(hv_cpu->synic_event_page);
> +
> + if (hv_cpu->synic_message_page != NULL)
> + memunmap(hv_cpu->synic_message_page);
> + } else {
> + free_page((unsigned long)hv_cpu->synic_event_page);
> + free_page((unsigned long)hv_cpu->synic_message_page);
> + }
> free_page((unsigned long)hv_cpu->post_msg_page);
> }
>
> @@ -213,10 +226,10 @@ void hv_synic_enable_regs(unsigned int cpu)
> union hv_synic_scontrol sctrl;
>
> /* Setup the Synic's message page */
> - simp.as_uint64 = hv_get_register(HV_REGISTER_SIMP);
> + simp.as_uint64 = hv_get_register(REG_SIMP);
To avoid all this code churn (here and in the next patch dealing with
EOM), would it make sense to move the logic picking nested/non-nested
register into hv_{get,set}_register() instead?
E.g. something like (untested, incomplete):
static inline u32 hv_get_nested_reg(u32 reg) {
switch (reg) {
HV_REGISTER_SIMP: return HV_REGISTER_NESTED_SIMP;
HV_REGISTER_NESTED_SVERSION: return HV_REGISTER_NESTED_SVERSION;
...
default: return reg;
}
}
static inline u64 hv_get_register(unsigned int reg)
{
u64 value;
if (hv_nested)
reg = hv_get_nested_reg(reg);
if (hv_is_synic_reg(reg) && hv_isolation_type_snp())
hv_ghcb_msr_read(reg, &value);
else
rdmsrl(reg, value);
return value;
}
static inline void hv_set_register(unsigned int reg, u64 value)
{
if (hv_nested)
reg = hv_get_nested_reg(reg);
if (hv_is_synic_reg(reg) && hv_isolation_type_snp()) {
hv_ghcb_msr_write(reg, value);
/* Write proxy bit via wrmsl instruction */
if (reg >= HV_REGISTER_SINT0 &&
reg <= HV_REGISTER_SINT15)
wrmsrl(reg, value | 1 << 20);
} else {
wrmsrl(reg, value);
}
}
> simp.simp_enabled = 1;
>
> - if (hv_isolation_type_snp()) {
> + if (hv_isolation_type_snp() || hv_root_partition) {
> hv_cpu->synic_message_page
> = memremap(simp.base_simp_gpa << HV_HYP_PAGE_SHIFT,
> HV_HYP_PAGE_SIZE, MEMREMAP_WB);
> @@ -227,13 +240,13 @@ void hv_synic_enable_regs(unsigned int cpu)
> >> HV_HYP_PAGE_SHIFT;
> }
>
> - hv_set_register(HV_REGISTER_SIMP, simp.as_uint64);
> + hv_set_register(REG_SIMP, simp.as_uint64);
>
> /* Setup the Synic's event page */
> - siefp.as_uint64 = hv_get_register(HV_REGISTER_SIEFP);
> + siefp.as_uint64 = hv_get_register(REG_SIEFP);
> siefp.siefp_enabled = 1;
>
> - if (hv_isolation_type_snp()) {
> + if (hv_isolation_type_snp() || hv_root_partition) {
> hv_cpu->synic_event_page =
> memremap(siefp.base_siefp_gpa << HV_HYP_PAGE_SHIFT,
> HV_HYP_PAGE_SIZE, MEMREMAP_WB);
> @@ -245,12 +258,12 @@ void hv_synic_enable_regs(unsigned int cpu)
> >> HV_HYP_PAGE_SHIFT;
> }
>
> - hv_set_register(HV_REGISTER_SIEFP, siefp.as_uint64);
> + hv_set_register(REG_SIEFP, siefp.as_uint64);
>
> /* Setup the shared SINT. */
> if (vmbus_irq != -1)
> enable_percpu_irq(vmbus_irq, 0);
> - shared_sint.as_uint64 = hv_get_register(HV_REGISTER_SINT0 +
> + shared_sint.as_uint64 = hv_get_register(REG_SINT0 +
> VMBUS_MESSAGE_SINT);
>
> shared_sint.vector = vmbus_interrupt;
> @@ -266,14 +279,14 @@ void hv_synic_enable_regs(unsigned int cpu)
> #else
> shared_sint.auto_eoi = 0;
> #endif
> - hv_set_register(HV_REGISTER_SINT0 + VMBUS_MESSAGE_SINT,
> + hv_set_register(REG_SINT0 + VMBUS_MESSAGE_SINT,
> shared_sint.as_uint64);
>
> /* Enable the global synic bit */
> - sctrl.as_uint64 = hv_get_register(HV_REGISTER_SCONTROL);
> + sctrl.as_uint64 = hv_get_register(REG_SCTRL);
> sctrl.enable = 1;
>
> - hv_set_register(HV_REGISTER_SCONTROL, sctrl.as_uint64);
> + hv_set_register(REG_SCTRL, sctrl.as_uint64);
> }
>
> int hv_synic_init(unsigned int cpu)
> @@ -297,17 +310,17 @@ void hv_synic_disable_regs(unsigned int cpu)
> union hv_synic_siefp siefp;
> union hv_synic_scontrol sctrl;
>
> - shared_sint.as_uint64 = hv_get_register(HV_REGISTER_SINT0 +
> + shared_sint.as_uint64 = hv_get_register(REG_SINT0 +
> VMBUS_MESSAGE_SINT);
>
> shared_sint.masked = 1;
>
> /* Need to correctly cleanup in the case of SMP!!! */
> /* Disable the interrupt */
> - hv_set_register(HV_REGISTER_SINT0 + VMBUS_MESSAGE_SINT,
> + hv_set_register(REG_SINT0 + VMBUS_MESSAGE_SINT,
> shared_sint.as_uint64);
>
> - simp.as_uint64 = hv_get_register(HV_REGISTER_SIMP);
> + simp.as_uint64 = hv_get_register(REG_SIMP);
> /*
> * In Isolation VM, sim and sief pages are allocated by
> * paravisor. These pages also will be used by kdump
> @@ -320,9 +333,9 @@ void hv_synic_disable_regs(unsigned int cpu)
> else
> simp.base_simp_gpa = 0;
>
> - hv_set_register(HV_REGISTER_SIMP, simp.as_uint64);
> + hv_set_register(REG_SIMP, simp.as_uint64);
>
> - siefp.as_uint64 = hv_get_register(HV_REGISTER_SIEFP);
> + siefp.as_uint64 = hv_get_register(REG_SIEFP);
> siefp.siefp_enabled = 0;
>
> if (hv_isolation_type_snp())
> @@ -330,12 +343,12 @@ void hv_synic_disable_regs(unsigned int cpu)
> else
> siefp.base_siefp_gpa = 0;
>
> - hv_set_register(HV_REGISTER_SIEFP, siefp.as_uint64);
> + hv_set_register(REG_SIEFP, siefp.as_uint64);
>
> /* Disable the global synic bit */
> - sctrl.as_uint64 = hv_get_register(HV_REGISTER_SCONTROL);
> + sctrl.as_uint64 = hv_get_register(REG_SCTRL);
> sctrl.enable = 0;
> - hv_set_register(HV_REGISTER_SCONTROL, sctrl.as_uint64);
> + hv_set_register(REG_SCTRL, sctrl.as_uint64);
>
> if (vmbus_irq != -1)
> disable_percpu_irq(vmbus_irq);
--
Vitaly
next prev parent reply other threads:[~2022-11-02 15:00 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-02 14:00 [PATCH 0/6] Add support running nested Microsoft Hypervisor Jinank Jain
2022-11-02 14:00 ` [PATCH 1/6] mshv: Add support for detecting nested hypervisor Jinank Jain
2022-11-02 14:00 ` [PATCH 2/6] hv: Setup synic registers in case of nested root partition Jinank Jain
2022-11-02 14:58 ` Vitaly Kuznetsov [this message]
2022-11-02 14:00 ` [PATCH 3/6] hv: Set the correct EOM register in case of nested hypervisor Jinank Jain
2022-11-02 14:00 ` [PATCH 4/6] hv: Add an interface to do nested hypercalls Jinank Jain
2022-11-02 14:00 ` [PATCH 5/6] hv: Enable vmbus driver for nested root partition Jinank Jain
2022-11-02 14:00 ` [PATCH 6/6] hv, mshv : Change interrupt vector " Jinank Jain
2022-11-02 15:46 ` Wei Liu
2022-11-24 5:53 ` [PATCH v5 0/5] Add support running nested Microsoft Hypervisor Jinank Jain
2022-11-24 5:53 ` [PATCH v5 1/5] x86/hyperv: Add support for detecting nested hypervisor Jinank Jain
2022-11-24 5:53 ` [PATCH v5 2/5] Drivers: hv: Setup synic registers in case of nested root partition Jinank Jain
2022-11-24 5:53 ` [PATCH v5 3/5] x86/hyperv: Add an interface to do nested hypercalls Jinank Jain
2022-11-24 5:53 ` [PATCH v5 4/5] Drivers: hv: Enable vmbus driver for nested root partition Jinank Jain
2022-11-24 5:53 ` [PATCH v5 5/5] x86/hyperv: Change interrupt vector " Jinank Jain
2022-11-24 6:01 ` [PATCH v5 0/5] Add support running nested Microsoft Hypervisor Jinank Jain
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=871qqls8rb.fsf@redhat.com \
--to=vkuznets@redhat.com \
--cc=ak@linux.intel.com \
--cc=arnd@arndb.de \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=decui@microsoft.com \
--cc=haiyangz@microsoft.com \
--cc=hpa@zytor.com \
--cc=jinankjain@linux.microsoft.com \
--cc=jpoimboe@kernel.org \
--cc=kirill.shutemov@linux.intel.com \
--cc=kys@microsoft.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-hyperv@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=sathyanarayanan.kuppuswamy@linux.intel.com \
--cc=seanjc@google.com \
--cc=sthemmin@microsoft.com \
--cc=tglx@linutronix.de \
--cc=wei.liu@kernel.org \
--cc=x86@kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.