* [PATCH v5 0/2] x86/jailhouse: improve probing of platform UARTs @ 2019-10-07 12:38 Ralf Ramsauer 2019-10-07 12:38 ` [PATCH v5 1/2] x86/jailhouse: improve setup data version comparison Ralf Ramsauer 2019-10-07 12:38 ` [PATCH v5 2/2] x86/jailhouse: Only enable platform UARTs if available Ralf Ramsauer 0 siblings, 2 replies; 11+ messages in thread From: Ralf Ramsauer @ 2019-10-07 12:38 UTC (permalink / raw) To: Jan Kiszka, Borislav Petkov, x86, jailhouse-dev, linux-kernel Cc: Ingo Molnar, H . Peter Anvin, Ralf Ramsauer Hi, probing of platform UARTs is a problem for x86 jailhouse non-root cells: Linux doesn't know which UARTs belong to the cell and will probe for all platform UARTs. This crashes the guest if access isn't permitted. Current workarounds (tuning via 8250.nr_uarts) are hacky and limited. But we do have some flags inside setup_data that indicate availability of UARTs, so simply use it. Ralf since v4: - Link: https://lore.kernel.org/r/20190909151030.152012-1-ralf.ramsauer@oth-regensburg.de alt: https://www.mail-archive.com/jailhouse-dev@googlegroups.com/msg07483.html - rebase and test on latest master and resolve conflicts - Add linux-kernel ML since v3: - Link: https://lore.kernel.org/r/20190819183408.988013-1-ralf.ramsauer@oth-regensburg.de alt: https://www.mail-archive.com/jailhouse-dev@googlegroups.com/msg07365.html - Address Thomas' comments (and it really looks nicer) - Address Jan's comment on patch 1 and add his Reviewed-by tag since v2: - Link: https://lore.kernel.org/r/20190812110650.631305-1-ralf.ramsauer@oth-regensburg.de alt: https://www.mail-archive.com/jailhouse-dev@googlegroups.com/msg07334.html - avoid imbalances of early_memremap and early_memunmap since v1: - Link: https://lore.kernel.org/r/20190802123333.4008-1-ralf.ramsauer@oth-regensburg.de - alt: https://www.mail-archive.com/jailhouse-dev@googlegroups.com/msg07283.html - setup data version check wasn't really prepared for extensions of the structure. Add a patch that improves the checks. Ralf Ramsauer (2): x86/jailhouse: improve setup data version comparison x86/jailhouse: Only enable platform UARTs if available arch/x86/include/uapi/asm/bootparam.h | 25 +++-- arch/x86/kernel/jailhouse.c | 131 ++++++++++++++++++++------ 2 files changed, 117 insertions(+), 39 deletions(-) -- 2.23.0 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v5 1/2] x86/jailhouse: improve setup data version comparison 2019-10-07 12:38 [PATCH v5 0/2] x86/jailhouse: improve probing of platform UARTs Ralf Ramsauer @ 2019-10-07 12:38 ` Ralf Ramsauer 2019-10-07 16:25 ` Borislav Petkov 2019-10-07 18:25 ` [tip: x86/platform] x86/jailhouse: Improve " tip-bot2 for Ralf Ramsauer 2019-10-07 12:38 ` [PATCH v5 2/2] x86/jailhouse: Only enable platform UARTs if available Ralf Ramsauer 1 sibling, 2 replies; 11+ messages in thread From: Ralf Ramsauer @ 2019-10-07 12:38 UTC (permalink / raw) To: Jan Kiszka, Borislav Petkov, x86, jailhouse-dev, linux-kernel Cc: Ingo Molnar, H . Peter Anvin, Ralf Ramsauer We will soon introduce a new setup_data version and extend the structure. This requires some preparational work for the sanity check of the header and the check of the version. Use the following strategy: 1. Ensure that the header declares at least enough space for the version and the compatible_version as we must hold that fields for any version. Furthermore, the location and semantics of those fields will never change. 2. Copy over data -- as much as we can. The length is either limited by the header length, or the length of setup_data. 3. Things are now in place -- sanity check if the header length complies the actual version. For future versions of the setup_data, only step 3 requires alignment. Signed-off-by: Ralf Ramsauer <ralf.ramsauer@oth-regensburg.de> Reviewed-by: Jan Kiszka <jan.kiszka@siemens.com> --- arch/x86/include/uapi/asm/bootparam.h | 22 +++++++----- arch/x86/kernel/jailhouse.c | 50 +++++++++++++++++---------- 2 files changed, 44 insertions(+), 28 deletions(-) diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h index c895df5482c5..43be437c9c71 100644 --- a/arch/x86/include/uapi/asm/bootparam.h +++ b/arch/x86/include/uapi/asm/bootparam.h @@ -139,15 +139,19 @@ struct boot_e820_entry { * setup data structure. */ struct jailhouse_setup_data { - __u16 version; - __u16 compatible_version; - __u16 pm_timer_address; - __u16 num_cpus; - __u64 pci_mmconfig_base; - __u32 tsc_khz; - __u32 apic_khz; - __u8 standard_ioapic; - __u8 cpu_ids[255]; + struct { + __u16 version; + __u16 compatible_version; + } __attribute__((packed)) hdr; + struct { + __u16 pm_timer_address; + __u16 num_cpus; + __u64 pci_mmconfig_base; + __u32 tsc_khz; + __u32 apic_khz; + __u8 standard_ioapic; + __u8 cpu_ids[255]; + } __attribute__((packed)) v1; } __attribute__((packed)); /* The so-called "zeropage" */ diff --git a/arch/x86/kernel/jailhouse.c b/arch/x86/kernel/jailhouse.c index 3ad34f01de2a..b9647add0063 100644 --- a/arch/x86/kernel/jailhouse.c +++ b/arch/x86/kernel/jailhouse.c @@ -22,6 +22,8 @@ #include <asm/jailhouse_para.h> static __initdata struct jailhouse_setup_data setup_data; +#define SETUP_DATA_V1_LEN (sizeof(setup_data.hdr) + sizeof(setup_data.v1)) + static unsigned int precalibrated_tsc_khz; static uint32_t jailhouse_cpuid_base(void) @@ -45,7 +47,7 @@ static void jailhouse_get_wallclock(struct timespec64 *now) static void __init jailhouse_timer_init(void) { - lapic_timer_period = setup_data.apic_khz * (1000 / HZ); + lapic_timer_period = setup_data.v1.apic_khz * (1000 / HZ); } static unsigned long jailhouse_get_tsc(void) @@ -88,14 +90,14 @@ static void __init jailhouse_get_smp_config(unsigned int early) register_lapic_address(0xfee00000); - for (cpu = 0; cpu < setup_data.num_cpus; cpu++) { - generic_processor_info(setup_data.cpu_ids[cpu], + for (cpu = 0; cpu < setup_data.v1.num_cpus; cpu++) { + generic_processor_info(setup_data.v1.cpu_ids[cpu], boot_cpu_apic_version); } smp_found_config = 1; - if (setup_data.standard_ioapic) { + if (setup_data.v1.standard_ioapic) { mp_register_ioapic(0, 0xfec00000, gsi_top, &ioapic_cfg); /* Register 1:1 mapping for legacy UART IRQs 3 and 4 */ @@ -126,9 +128,9 @@ static int __init jailhouse_pci_arch_init(void) pcibios_last_bus = 0xff; #ifdef CONFIG_PCI_MMCONFIG - if (setup_data.pci_mmconfig_base) { + if (setup_data.v1.pci_mmconfig_base) { pci_mmconfig_add(0, 0, pcibios_last_bus, - setup_data.pci_mmconfig_base); + setup_data.v1.pci_mmconfig_base); pci_mmcfg_arch_init(); } #endif @@ -139,6 +141,7 @@ static int __init jailhouse_pci_arch_init(void) static void __init jailhouse_init_platform(void) { u64 pa_data = boot_params.hdr.setup_data; + unsigned long setup_data_len; struct setup_data header; void *mapping; @@ -163,16 +166,8 @@ static void __init jailhouse_init_platform(void) memcpy(&header, mapping, sizeof(header)); early_memunmap(mapping, sizeof(header)); - if (header.type == SETUP_JAILHOUSE && - header.len >= sizeof(setup_data)) { - pa_data += offsetof(struct setup_data, data); - - mapping = early_memremap(pa_data, sizeof(setup_data)); - memcpy(&setup_data, mapping, sizeof(setup_data)); - early_memunmap(mapping, sizeof(setup_data)); - + if (header.type == SETUP_JAILHOUSE) break; - } pa_data = header.next; } @@ -180,13 +175,26 @@ static void __init jailhouse_init_platform(void) if (!pa_data) panic("Jailhouse: No valid setup data found"); - if (setup_data.compatible_version > JAILHOUSE_SETUP_REQUIRED_VERSION) - panic("Jailhouse: Unsupported setup data structure"); + /* setup data must at least contain the header */ + if (header.len < sizeof(setup_data.hdr)) + goto unsupported; - pmtmr_ioport = setup_data.pm_timer_address; + pa_data += offsetof(struct setup_data, data); + setup_data_len = min(sizeof(setup_data), (unsigned long)header.len); + mapping = early_memremap(pa_data, setup_data_len); + memcpy(&setup_data, mapping, setup_data_len); + early_memunmap(mapping, setup_data_len); + + if (setup_data.hdr.version == 0 || + setup_data.hdr.compatible_version != + JAILHOUSE_SETUP_REQUIRED_VERSION || + (setup_data.hdr.version >= 1 && header.len < SETUP_DATA_V1_LEN)) + goto unsupported; + + pmtmr_ioport = setup_data.v1.pm_timer_address; pr_debug("Jailhouse: PM-Timer IO Port: %#x\n", pmtmr_ioport); - precalibrated_tsc_khz = setup_data.tsc_khz; + precalibrated_tsc_khz = setup_data.v1.tsc_khz; setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ); pci_probe = 0; @@ -196,6 +204,10 @@ static void __init jailhouse_init_platform(void) * are none in a non-root cell. */ disable_acpi(); + return; + +unsupported: + panic("Jailhouse: Unsupported setup data structure"); } bool jailhouse_paravirt(void) -- 2.23.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v5 1/2] x86/jailhouse: improve setup data version comparison 2019-10-07 12:38 ` [PATCH v5 1/2] x86/jailhouse: improve setup data version comparison Ralf Ramsauer @ 2019-10-07 16:25 ` Borislav Petkov 2019-10-07 18:25 ` [tip: x86/platform] x86/jailhouse: Improve " tip-bot2 for Ralf Ramsauer 1 sibling, 0 replies; 11+ messages in thread From: Borislav Petkov @ 2019-10-07 16:25 UTC (permalink / raw) To: Ralf Ramsauer Cc: Jan Kiszka, x86, jailhouse-dev, linux-kernel, Ingo Molnar, H . Peter Anvin On Mon, Oct 07, 2019 at 02:38:18PM +0200, Ralf Ramsauer wrote: > We will soon introduce a new setup_data version and extend the Who is "We"? There a couple more "we" below. Can you please rewrite that commit message in passive voice and thus dispense my confusion about who's "we"? :) > structure. This requires some preparational work for the sanity check of > the header and the check of the version. > > Use the following strategy: > > 1. Ensure that the header declares at least enough space for the version > and the compatible_version as we must hold that fields for any > version. Furthermore, the location and semantics of those fields will > never change. > > 2. Copy over data -- as much as we can. The length is either limited by > the header length, or the length of setup_data. > > 3. Things are now in place -- sanity check if the header length complies > the actual version. > > For future versions of the setup_data, only step 3 requires alignment. > > Signed-off-by: Ralf Ramsauer <ralf.ramsauer@oth-regensburg.de> > Reviewed-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > arch/x86/include/uapi/asm/bootparam.h | 22 +++++++----- > arch/x86/kernel/jailhouse.c | 50 +++++++++++++++++---------- > 2 files changed, 44 insertions(+), 28 deletions(-) ... > @@ -180,13 +175,26 @@ static void __init jailhouse_init_platform(void) > if (!pa_data) > panic("Jailhouse: No valid setup data found"); > > - if (setup_data.compatible_version > JAILHOUSE_SETUP_REQUIRED_VERSION) > - panic("Jailhouse: Unsupported setup data structure"); > + /* setup data must at least contain the header */ > + if (header.len < sizeof(setup_data.hdr)) > + goto unsupported; > > - pmtmr_ioport = setup_data.pm_timer_address; > + pa_data += offsetof(struct setup_data, data); > + setup_data_len = min(sizeof(setup_data), (unsigned long)header.len); Checkpatch makes sense here: WARNING: min() should probably be min_t(unsigned long, sizeof(setup_data), header.len) #165: FILE: arch/x86/kernel/jailhouse.c:183: + setup_data_len = min(sizeof(setup_data), (unsigned long)header.len); -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 11+ messages in thread
* [tip: x86/platform] x86/jailhouse: Improve setup data version comparison 2019-10-07 12:38 ` [PATCH v5 1/2] x86/jailhouse: improve setup data version comparison Ralf Ramsauer 2019-10-07 16:25 ` Borislav Petkov @ 2019-10-07 18:25 ` tip-bot2 for Ralf Ramsauer 1 sibling, 0 replies; 11+ messages in thread From: tip-bot2 for Ralf Ramsauer @ 2019-10-07 18:25 UTC (permalink / raw) To: linux-tip-commits Cc: Ralf Ramsauer, Jan Kiszka, Borislav Petkov, H . Peter Anvin, Linus Torvalds, Peter Zijlstra, Thomas Gleixner, jailhouse-dev, Ingo Molnar, linux-kernel The following commit has been merged into the x86/platform branch of tip: Commit-ID: a980bfcccf1ebd7cc404c381ca747191e17bc28f Gitweb: https://git.kernel.org/tip/a980bfcccf1ebd7cc404c381ca747191e17bc28f Author: Ralf Ramsauer <ralf.ramsauer@oth-regensburg.de> AuthorDate: Mon, 07 Oct 2019 14:38:18 +02:00 Committer: Ingo Molnar <mingo@kernel.org> CommitterDate: Mon, 07 Oct 2019 17:35:50 +02:00 x86/jailhouse: Improve setup data version comparison We will soon introduce a new setup_data version and extend the structure. This requires some preparational work for the sanity check of the header and the check of the version. Use the following strategy: 1. Ensure that the header declares at least enough space for the version and the compatible_version as we must hold that fields for any version. Furthermore, the location and semantics of those fields will never change. 2. Copy over data -- as much as we can. The length is either limited by the header length, or the length of setup_data. 3. Things are now in place -- sanity check if the header length complies the actual version. For future versions of the setup_data, only step 3 requires alignment. Signed-off-by: Ralf Ramsauer <ralf.ramsauer@oth-regensburg.de> Reviewed-by: Jan Kiszka <jan.kiszka@siemens.com> Cc: Borislav Petkov <bp@alien8.de> Cc: H . Peter Anvin <hpa@zytor.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: jailhouse-dev@googlegroups.com Link: https://lkml.kernel.org/r/20191007123819.161432-2-ralf.ramsauer@oth-regensburg.de Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/include/uapi/asm/bootparam.h | 22 ++++++----- arch/x86/kernel/jailhouse.c | 50 ++++++++++++++++---------- 2 files changed, 44 insertions(+), 28 deletions(-) diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h index c895df5..43be437 100644 --- a/arch/x86/include/uapi/asm/bootparam.h +++ b/arch/x86/include/uapi/asm/bootparam.h @@ -139,15 +139,19 @@ struct boot_e820_entry { * setup data structure. */ struct jailhouse_setup_data { - __u16 version; - __u16 compatible_version; - __u16 pm_timer_address; - __u16 num_cpus; - __u64 pci_mmconfig_base; - __u32 tsc_khz; - __u32 apic_khz; - __u8 standard_ioapic; - __u8 cpu_ids[255]; + struct { + __u16 version; + __u16 compatible_version; + } __attribute__((packed)) hdr; + struct { + __u16 pm_timer_address; + __u16 num_cpus; + __u64 pci_mmconfig_base; + __u32 tsc_khz; + __u32 apic_khz; + __u8 standard_ioapic; + __u8 cpu_ids[255]; + } __attribute__((packed)) v1; } __attribute__((packed)); /* The so-called "zeropage" */ diff --git a/arch/x86/kernel/jailhouse.c b/arch/x86/kernel/jailhouse.c index 3ad34f0..b9647ad 100644 --- a/arch/x86/kernel/jailhouse.c +++ b/arch/x86/kernel/jailhouse.c @@ -22,6 +22,8 @@ #include <asm/jailhouse_para.h> static __initdata struct jailhouse_setup_data setup_data; +#define SETUP_DATA_V1_LEN (sizeof(setup_data.hdr) + sizeof(setup_data.v1)) + static unsigned int precalibrated_tsc_khz; static uint32_t jailhouse_cpuid_base(void) @@ -45,7 +47,7 @@ static void jailhouse_get_wallclock(struct timespec64 *now) static void __init jailhouse_timer_init(void) { - lapic_timer_period = setup_data.apic_khz * (1000 / HZ); + lapic_timer_period = setup_data.v1.apic_khz * (1000 / HZ); } static unsigned long jailhouse_get_tsc(void) @@ -88,14 +90,14 @@ static void __init jailhouse_get_smp_config(unsigned int early) register_lapic_address(0xfee00000); - for (cpu = 0; cpu < setup_data.num_cpus; cpu++) { - generic_processor_info(setup_data.cpu_ids[cpu], + for (cpu = 0; cpu < setup_data.v1.num_cpus; cpu++) { + generic_processor_info(setup_data.v1.cpu_ids[cpu], boot_cpu_apic_version); } smp_found_config = 1; - if (setup_data.standard_ioapic) { + if (setup_data.v1.standard_ioapic) { mp_register_ioapic(0, 0xfec00000, gsi_top, &ioapic_cfg); /* Register 1:1 mapping for legacy UART IRQs 3 and 4 */ @@ -126,9 +128,9 @@ static int __init jailhouse_pci_arch_init(void) pcibios_last_bus = 0xff; #ifdef CONFIG_PCI_MMCONFIG - if (setup_data.pci_mmconfig_base) { + if (setup_data.v1.pci_mmconfig_base) { pci_mmconfig_add(0, 0, pcibios_last_bus, - setup_data.pci_mmconfig_base); + setup_data.v1.pci_mmconfig_base); pci_mmcfg_arch_init(); } #endif @@ -139,6 +141,7 @@ static int __init jailhouse_pci_arch_init(void) static void __init jailhouse_init_platform(void) { u64 pa_data = boot_params.hdr.setup_data; + unsigned long setup_data_len; struct setup_data header; void *mapping; @@ -163,16 +166,8 @@ static void __init jailhouse_init_platform(void) memcpy(&header, mapping, sizeof(header)); early_memunmap(mapping, sizeof(header)); - if (header.type == SETUP_JAILHOUSE && - header.len >= sizeof(setup_data)) { - pa_data += offsetof(struct setup_data, data); - - mapping = early_memremap(pa_data, sizeof(setup_data)); - memcpy(&setup_data, mapping, sizeof(setup_data)); - early_memunmap(mapping, sizeof(setup_data)); - + if (header.type == SETUP_JAILHOUSE) break; - } pa_data = header.next; } @@ -180,13 +175,26 @@ static void __init jailhouse_init_platform(void) if (!pa_data) panic("Jailhouse: No valid setup data found"); - if (setup_data.compatible_version > JAILHOUSE_SETUP_REQUIRED_VERSION) - panic("Jailhouse: Unsupported setup data structure"); + /* setup data must at least contain the header */ + if (header.len < sizeof(setup_data.hdr)) + goto unsupported; - pmtmr_ioport = setup_data.pm_timer_address; + pa_data += offsetof(struct setup_data, data); + setup_data_len = min(sizeof(setup_data), (unsigned long)header.len); + mapping = early_memremap(pa_data, setup_data_len); + memcpy(&setup_data, mapping, setup_data_len); + early_memunmap(mapping, setup_data_len); + + if (setup_data.hdr.version == 0 || + setup_data.hdr.compatible_version != + JAILHOUSE_SETUP_REQUIRED_VERSION || + (setup_data.hdr.version >= 1 && header.len < SETUP_DATA_V1_LEN)) + goto unsupported; + + pmtmr_ioport = setup_data.v1.pm_timer_address; pr_debug("Jailhouse: PM-Timer IO Port: %#x\n", pmtmr_ioport); - precalibrated_tsc_khz = setup_data.tsc_khz; + precalibrated_tsc_khz = setup_data.v1.tsc_khz; setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ); pci_probe = 0; @@ -196,6 +204,10 @@ static void __init jailhouse_init_platform(void) * are none in a non-root cell. */ disable_acpi(); + return; + +unsupported: + panic("Jailhouse: Unsupported setup data structure"); } bool jailhouse_paravirt(void) ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v5 2/2] x86/jailhouse: Only enable platform UARTs if available 2019-10-07 12:38 [PATCH v5 0/2] x86/jailhouse: improve probing of platform UARTs Ralf Ramsauer 2019-10-07 12:38 ` [PATCH v5 1/2] x86/jailhouse: improve setup data version comparison Ralf Ramsauer @ 2019-10-07 12:38 ` Ralf Ramsauer 2019-10-07 15:20 ` Jan Kiszka ` (2 more replies) 1 sibling, 3 replies; 11+ messages in thread From: Ralf Ramsauer @ 2019-10-07 12:38 UTC (permalink / raw) To: Jan Kiszka, Borislav Petkov, x86, jailhouse-dev, linux-kernel Cc: Ingo Molnar, H . Peter Anvin, Ralf Ramsauer ACPI tables aren't available if Linux runs as guest of the hypervisor Jailhouse. This makes the 8250 driver probe for all platform UARTs as it assumes that all platform are present in case of !ACPI. Jailhouse will stop execution of Linux guest due to port access violation. So far, these access violations could be solved by tuning the 8250.nr_uarts parameter but it has limitations: We can, e.g., only map consecutive platform UARTs to Linux, and only in the sequence 0x3f8, 0x2f8, 0x3e8, 0x2e8. Beginning from setup_data version 2, Jailhouse will place information of available platform UARTs in setup_data. This allows for selective activation of platform UARTs. This patch queries the setup_data version and activates only available UARTS. It comes with backward compatibility, and will still support older setup_data versions. In this case, Linux falls back to the old behaviour. Signed-off-by: Ralf Ramsauer <ralf.ramsauer@oth-regensburg.de> --- arch/x86/include/uapi/asm/bootparam.h | 3 + arch/x86/kernel/jailhouse.c | 83 +++++++++++++++++++++++---- 2 files changed, 74 insertions(+), 12 deletions(-) diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h index 43be437c9c71..db1e24e56e94 100644 --- a/arch/x86/include/uapi/asm/bootparam.h +++ b/arch/x86/include/uapi/asm/bootparam.h @@ -152,6 +152,9 @@ struct jailhouse_setup_data { __u8 standard_ioapic; __u8 cpu_ids[255]; } __attribute__((packed)) v1; + struct { + __u32 flags; + } __attribute__((packed)) v2; } __attribute__((packed)); /* The so-called "zeropage" */ diff --git a/arch/x86/kernel/jailhouse.c b/arch/x86/kernel/jailhouse.c index b9647add0063..95550c08ab23 100644 --- a/arch/x86/kernel/jailhouse.c +++ b/arch/x86/kernel/jailhouse.c @@ -11,6 +11,7 @@ #include <linux/acpi_pmtmr.h> #include <linux/kernel.h> #include <linux/reboot.h> +#include <linux/serial_8250.h> #include <asm/apic.h> #include <asm/cpu.h> #include <asm/hypervisor.h> @@ -23,9 +24,22 @@ static __initdata struct jailhouse_setup_data setup_data; #define SETUP_DATA_V1_LEN (sizeof(setup_data.hdr) + sizeof(setup_data.v1)) +#define SETUP_DATA_V2_LEN (SETUP_DATA_V1_LEN + sizeof(setup_data.v2)) static unsigned int precalibrated_tsc_khz; +static void jailhouse_setup_irq(unsigned int irq) +{ + struct mpc_intsrc mp_irq = { + .type = MP_INTSRC, + .irqtype = mp_INT, + .irqflag = MP_IRQPOL_ACTIVE_HIGH | MP_IRQTRIG_EDGE, + .srcbusirq = irq, + .dstirq = irq, + }; + mp_save_irq(&mp_irq); +} + static uint32_t jailhouse_cpuid_base(void) { if (boot_cpu_data.cpuid_level < 0 || @@ -79,11 +93,6 @@ static void __init jailhouse_get_smp_config(unsigned int early) .type = IOAPIC_DOMAIN_STRICT, .ops = &mp_ioapic_irqdomain_ops, }; - struct mpc_intsrc mp_irq = { - .type = MP_INTSRC, - .irqtype = mp_INT, - .irqflag = MP_IRQPOL_ACTIVE_HIGH | MP_IRQTRIG_EDGE, - }; unsigned int cpu; jailhouse_x2apic_init(); @@ -100,12 +109,12 @@ static void __init jailhouse_get_smp_config(unsigned int early) if (setup_data.v1.standard_ioapic) { mp_register_ioapic(0, 0xfec00000, gsi_top, &ioapic_cfg); - /* Register 1:1 mapping for legacy UART IRQs 3 and 4 */ - mp_irq.srcbusirq = mp_irq.dstirq = 3; - mp_save_irq(&mp_irq); - - mp_irq.srcbusirq = mp_irq.dstirq = 4; - mp_save_irq(&mp_irq); + if (IS_ENABLED(CONFIG_SERIAL_8250) && + setup_data.hdr.version < 2) { + /* Register 1:1 mapping for legacy UART IRQs 3 and 4 */ + jailhouse_setup_irq(3); + jailhouse_setup_irq(4); + } } } @@ -138,6 +147,53 @@ static int __init jailhouse_pci_arch_init(void) return 0; } +#ifdef CONFIG_SERIAL_8250 +static bool jailhouse_uart_enabled(unsigned int uart_nr) +{ + return setup_data.v2.flags & BIT(uart_nr); +} + +static void jailhouse_serial_fixup(int port, struct uart_port *up, + u32 *capabilities) +{ + static const u16 pcuart_base[] = {0x3f8, 0x2f8, 0x3e8, 0x2e8}; + unsigned int n; + + for (n = 0; n < ARRAY_SIZE(pcuart_base); n++) { + if (pcuart_base[n] != up->iobase) + continue; + + if (jailhouse_uart_enabled(n)) { + pr_info("Enabling UART%u (port 0x%lx)\n", n, + up->iobase); + jailhouse_setup_irq(up->irq); + } else { + /* Deactivate UART if access isn't allowed */ + up->iobase = 0; + } + break; + } +} + +static void jailhouse_serial_workaround(void) +{ + /* + * There are flags inside setup_data that indicate availability of + * platform UARTs since setup data version 2. + * + * In case of version 1, we don't know which UARTs belong Linux. In + * this case, unconditionally register 1:1 mapping for legacy UART IRQs + * 3 and 4. + */ + if (setup_data.hdr.version > 1) + serial8250_set_isa_configurator(jailhouse_serial_fixup); +} +#else /* !CONFIG_SERIAL_8250 */ +static inline void jailhouse_serial_workaround(void) +{ +} +#endif /* CONFIG_SERIAL_8250 */ + static void __init jailhouse_init_platform(void) { u64 pa_data = boot_params.hdr.setup_data; @@ -188,7 +244,8 @@ static void __init jailhouse_init_platform(void) if (setup_data.hdr.version == 0 || setup_data.hdr.compatible_version != JAILHOUSE_SETUP_REQUIRED_VERSION || - (setup_data.hdr.version >= 1 && header.len < SETUP_DATA_V1_LEN)) + (setup_data.hdr.version == 1 && header.len < SETUP_DATA_V1_LEN) || + (setup_data.hdr.version >= 2 && header.len < SETUP_DATA_V2_LEN)) goto unsupported; pmtmr_ioport = setup_data.v1.pm_timer_address; @@ -204,6 +261,8 @@ static void __init jailhouse_init_platform(void) * are none in a non-root cell. */ disable_acpi(); + + jailhouse_serial_workaround(); return; unsupported: -- 2.23.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v5 2/2] x86/jailhouse: Only enable platform UARTs if available 2019-10-07 12:38 ` [PATCH v5 2/2] x86/jailhouse: Only enable platform UARTs if available Ralf Ramsauer @ 2019-10-07 15:20 ` Jan Kiszka 2019-10-07 16:26 ` Borislav Petkov 2019-10-07 18:25 ` [tip: x86/platform] x86/jailhouse: Enable platform UARTs only " tip-bot2 for Ralf Ramsauer 2 siblings, 0 replies; 11+ messages in thread From: Jan Kiszka @ 2019-10-07 15:20 UTC (permalink / raw) To: Ralf Ramsauer, Borislav Petkov, x86, jailhouse-dev, linux-kernel Cc: Ingo Molnar, H . Peter Anvin On 07.10.19 14:38, Ralf Ramsauer wrote: > ACPI tables aren't available if Linux runs as guest of the hypervisor > Jailhouse. This makes the 8250 driver probe for all platform UARTs as > it assumes that all platform are present in case of !ACPI. Jailhouse > will stop execution of Linux guest due to port access violation. > > So far, these access violations could be solved by tuning the > 8250.nr_uarts parameter but it has limitations: We can, e.g., only map > consecutive platform UARTs to Linux, and only in the sequence 0x3f8, > 0x2f8, 0x3e8, 0x2e8. > > Beginning from setup_data version 2, Jailhouse will place information of > available platform UARTs in setup_data. This allows for selective > activation of platform UARTs. > > This patch queries the setup_data version and activates only available > UARTS. It comes with backward compatibility, and will still support > older setup_data versions. In this case, Linux falls back to the old > behaviour. > > Signed-off-by: Ralf Ramsauer <ralf.ramsauer@oth-regensburg.de> > --- > arch/x86/include/uapi/asm/bootparam.h | 3 + > arch/x86/kernel/jailhouse.c | 83 +++++++++++++++++++++++---- > 2 files changed, 74 insertions(+), 12 deletions(-) > > diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h > index 43be437c9c71..db1e24e56e94 100644 > --- a/arch/x86/include/uapi/asm/bootparam.h > +++ b/arch/x86/include/uapi/asm/bootparam.h > @@ -152,6 +152,9 @@ struct jailhouse_setup_data { > __u8 standard_ioapic; > __u8 cpu_ids[255]; > } __attribute__((packed)) v1; > + struct { > + __u32 flags; > + } __attribute__((packed)) v2; > } __attribute__((packed)); > > /* The so-called "zeropage" */ > diff --git a/arch/x86/kernel/jailhouse.c b/arch/x86/kernel/jailhouse.c > index b9647add0063..95550c08ab23 100644 > --- a/arch/x86/kernel/jailhouse.c > +++ b/arch/x86/kernel/jailhouse.c > @@ -11,6 +11,7 @@ > #include <linux/acpi_pmtmr.h> > #include <linux/kernel.h> > #include <linux/reboot.h> > +#include <linux/serial_8250.h> > #include <asm/apic.h> > #include <asm/cpu.h> > #include <asm/hypervisor.h> > @@ -23,9 +24,22 @@ > > static __initdata struct jailhouse_setup_data setup_data; > #define SETUP_DATA_V1_LEN (sizeof(setup_data.hdr) + sizeof(setup_data.v1)) > +#define SETUP_DATA_V2_LEN (SETUP_DATA_V1_LEN + sizeof(setup_data.v2)) > > static unsigned int precalibrated_tsc_khz; > > +static void jailhouse_setup_irq(unsigned int irq) > +{ > + struct mpc_intsrc mp_irq = { > + .type = MP_INTSRC, > + .irqtype = mp_INT, > + .irqflag = MP_IRQPOL_ACTIVE_HIGH | MP_IRQTRIG_EDGE, > + .srcbusirq = irq, > + .dstirq = irq, > + }; > + mp_save_irq(&mp_irq); > +} > + > static uint32_t jailhouse_cpuid_base(void) > { > if (boot_cpu_data.cpuid_level < 0 || > @@ -79,11 +93,6 @@ static void __init jailhouse_get_smp_config(unsigned int early) > .type = IOAPIC_DOMAIN_STRICT, > .ops = &mp_ioapic_irqdomain_ops, > }; > - struct mpc_intsrc mp_irq = { > - .type = MP_INTSRC, > - .irqtype = mp_INT, > - .irqflag = MP_IRQPOL_ACTIVE_HIGH | MP_IRQTRIG_EDGE, > - }; > unsigned int cpu; > > jailhouse_x2apic_init(); > @@ -100,12 +109,12 @@ static void __init jailhouse_get_smp_config(unsigned int early) > if (setup_data.v1.standard_ioapic) { > mp_register_ioapic(0, 0xfec00000, gsi_top, &ioapic_cfg); > > - /* Register 1:1 mapping for legacy UART IRQs 3 and 4 */ > - mp_irq.srcbusirq = mp_irq.dstirq = 3; > - mp_save_irq(&mp_irq); > - > - mp_irq.srcbusirq = mp_irq.dstirq = 4; > - mp_save_irq(&mp_irq); > + if (IS_ENABLED(CONFIG_SERIAL_8250) && > + setup_data.hdr.version < 2) { > + /* Register 1:1 mapping for legacy UART IRQs 3 and 4 */ > + jailhouse_setup_irq(3); > + jailhouse_setup_irq(4); > + } > } > } > > @@ -138,6 +147,53 @@ static int __init jailhouse_pci_arch_init(void) > return 0; > } > > +#ifdef CONFIG_SERIAL_8250 > +static bool jailhouse_uart_enabled(unsigned int uart_nr) > +{ > + return setup_data.v2.flags & BIT(uart_nr); > +} > + > +static void jailhouse_serial_fixup(int port, struct uart_port *up, > + u32 *capabilities) > +{ > + static const u16 pcuart_base[] = {0x3f8, 0x2f8, 0x3e8, 0x2e8}; > + unsigned int n; > + > + for (n = 0; n < ARRAY_SIZE(pcuart_base); n++) { > + if (pcuart_base[n] != up->iobase) > + continue; > + > + if (jailhouse_uart_enabled(n)) { > + pr_info("Enabling UART%u (port 0x%lx)\n", n, > + up->iobase); > + jailhouse_setup_irq(up->irq); > + } else { > + /* Deactivate UART if access isn't allowed */ > + up->iobase = 0; > + } > + break; > + } > +} > + > +static void jailhouse_serial_workaround(void) > +{ > + /* > + * There are flags inside setup_data that indicate availability of > + * platform UARTs since setup data version 2. > + * > + * In case of version 1, we don't know which UARTs belong Linux. In > + * this case, unconditionally register 1:1 mapping for legacy UART IRQs > + * 3 and 4. > + */ > + if (setup_data.hdr.version > 1) > + serial8250_set_isa_configurator(jailhouse_serial_fixup); > +} > +#else /* !CONFIG_SERIAL_8250 */ > +static inline void jailhouse_serial_workaround(void) > +{ > +} > +#endif /* CONFIG_SERIAL_8250 */ > + > static void __init jailhouse_init_platform(void) > { > u64 pa_data = boot_params.hdr.setup_data; > @@ -188,7 +244,8 @@ static void __init jailhouse_init_platform(void) > if (setup_data.hdr.version == 0 || > setup_data.hdr.compatible_version != > JAILHOUSE_SETUP_REQUIRED_VERSION || > - (setup_data.hdr.version >= 1 && header.len < SETUP_DATA_V1_LEN)) > + (setup_data.hdr.version == 1 && header.len < SETUP_DATA_V1_LEN) || > + (setup_data.hdr.version >= 2 && header.len < SETUP_DATA_V2_LEN)) > goto unsupported; > > pmtmr_ioport = setup_data.v1.pm_timer_address; > @@ -204,6 +261,8 @@ static void __init jailhouse_init_platform(void) > * are none in a non-root cell. > */ > disable_acpi(); > + > + jailhouse_serial_workaround(); > return; > > unsupported: > This was missing yet: Reviewed-by: Jan Kiszka <jan.kiszka@siemens.com> Thanks, Jan -- Siemens AG, Corporate Technology, CT RDA IOT SES-DE Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 2/2] x86/jailhouse: Only enable platform UARTs if available 2019-10-07 12:38 ` [PATCH v5 2/2] x86/jailhouse: Only enable platform UARTs if available Ralf Ramsauer 2019-10-07 15:20 ` Jan Kiszka @ 2019-10-07 16:26 ` Borislav Petkov 2019-10-07 16:44 ` Ralf Ramsauer 2019-10-07 18:25 ` [tip: x86/platform] x86/jailhouse: Enable platform UARTs only " tip-bot2 for Ralf Ramsauer 2 siblings, 1 reply; 11+ messages in thread From: Borislav Petkov @ 2019-10-07 16:26 UTC (permalink / raw) To: Ralf Ramsauer Cc: Jan Kiszka, x86, jailhouse-dev, linux-kernel, Ingo Molnar, H . Peter Anvin On Mon, Oct 07, 2019 at 02:38:19PM +0200, Ralf Ramsauer wrote: > ACPI tables aren't available if Linux runs as guest of the hypervisor > Jailhouse. This makes the 8250 driver probe for all platform UARTs as > it assumes that all platform are present in case of !ACPI. Jailhouse I think you mean s/platform/UARTs/ here. > will stop execution of Linux guest due to port access violation. > > So far, these access violations could be solved by tuning the > 8250.nr_uarts parameter but it has limitations: We can, e.g., only map Another "We" you can get rid of. > consecutive platform UARTs to Linux, and only in the sequence 0x3f8, > 0x2f8, 0x3e8, 0x2e8. > > Beginning from setup_data version 2, Jailhouse will place information of > available platform UARTs in setup_data. This allows for selective > activation of platform UARTs. > > This patch queries the setup_data version and activates only available s/This patch queries/Query/ > UARTS. It comes with backward compatibility, and will still support > older setup_data versions. In this case, Linux falls back to the old > behaviour. > > Signed-off-by: Ralf Ramsauer <ralf.ramsauer@oth-regensburg.de> > --- > arch/x86/include/uapi/asm/bootparam.h | 3 + > arch/x86/kernel/jailhouse.c | 83 +++++++++++++++++++++++---- > 2 files changed, 74 insertions(+), 12 deletions(-) ... > @@ -138,6 +147,53 @@ static int __init jailhouse_pci_arch_init(void) > return 0; > } > > +#ifdef CONFIG_SERIAL_8250 > +static bool jailhouse_uart_enabled(unsigned int uart_nr) > +{ > + return setup_data.v2.flags & BIT(uart_nr); > +} > + > +static void jailhouse_serial_fixup(int port, struct uart_port *up, > + u32 *capabilities) > +{ > + static const u16 pcuart_base[] = {0x3f8, 0x2f8, 0x3e8, 0x2e8}; > + unsigned int n; > + > + for (n = 0; n < ARRAY_SIZE(pcuart_base); n++) { > + if (pcuart_base[n] != up->iobase) > + continue; > + > + if (jailhouse_uart_enabled(n)) { > + pr_info("Enabling UART%u (port 0x%lx)\n", n, > + up->iobase); > + jailhouse_setup_irq(up->irq); > + } else { > + /* Deactivate UART if access isn't allowed */ > + up->iobase = 0; > + } > + break; > + } > +} WARNING: vmlinux.o(.text+0x4fdb0): Section mismatch in reference from the function jailhouse_serial_fixup() to the variable .init.data:can_use_brk_pgt The function jailhouse_serial_fixup() references the variable __initdata can_use_brk_pgt. This is often because jailhouse_serial_fixup lacks a __initdata annotation or the annotation of can_use_brk_pgt is wrong. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 2/2] x86/jailhouse: Only enable platform UARTs if available 2019-10-07 16:26 ` Borislav Petkov @ 2019-10-07 16:44 ` Ralf Ramsauer 2019-10-07 16:59 ` Borislav Petkov 0 siblings, 1 reply; 11+ messages in thread From: Ralf Ramsauer @ 2019-10-07 16:44 UTC (permalink / raw) To: Borislav Petkov Cc: Jan Kiszka, x86, jailhouse-dev, linux-kernel, Ingo Molnar, H . Peter Anvin On 10/7/19 6:26 PM, Borislav Petkov wrote: > On Mon, Oct 07, 2019 at 02:38:19PM +0200, Ralf Ramsauer wrote: >> ACPI tables aren't available if Linux runs as guest of the hypervisor >> Jailhouse. This makes the 8250 driver probe for all platform UARTs as >> it assumes that all platform are present in case of !ACPI. Jailhouse > > I think you mean s/platform/UARTs/ here. > >> will stop execution of Linux guest due to port access violation. >> >> So far, these access violations could be solved by tuning the >> 8250.nr_uarts parameter but it has limitations: We can, e.g., only map > > Another "We" you can get rid of. > >> consecutive platform UARTs to Linux, and only in the sequence 0x3f8, >> 0x2f8, 0x3e8, 0x2e8. >> >> Beginning from setup_data version 2, Jailhouse will place information of >> available platform UARTs in setup_data. This allows for selective >> activation of platform UARTs. >> >> This patch queries the setup_data version and activates only available > > s/This patch queries/Query/ > >> UARTS. It comes with backward compatibility, and will still support >> older setup_data versions. In this case, Linux falls back to the old >> behaviour. >> >> Signed-off-by: Ralf Ramsauer <ralf.ramsauer@oth-regensburg.de> >> --- >> arch/x86/include/uapi/asm/bootparam.h | 3 + >> arch/x86/kernel/jailhouse.c | 83 +++++++++++++++++++++++---- >> 2 files changed, 74 insertions(+), 12 deletions(-) > > ... > >> @@ -138,6 +147,53 @@ static int __init jailhouse_pci_arch_init(void) >> return 0; >> } >> >> +#ifdef CONFIG_SERIAL_8250 >> +static bool jailhouse_uart_enabled(unsigned int uart_nr) >> +{ >> + return setup_data.v2.flags & BIT(uart_nr); >> +} >> + >> +static void jailhouse_serial_fixup(int port, struct uart_port *up, >> + u32 *capabilities) >> +{ >> + static const u16 pcuart_base[] = {0x3f8, 0x2f8, 0x3e8, 0x2e8}; >> + unsigned int n; >> + >> + for (n = 0; n < ARRAY_SIZE(pcuart_base); n++) { >> + if (pcuart_base[n] != up->iobase) >> + continue; >> + >> + if (jailhouse_uart_enabled(n)) { >> + pr_info("Enabling UART%u (port 0x%lx)\n", n, >> + up->iobase); >> + jailhouse_setup_irq(up->irq); >> + } else { >> + /* Deactivate UART if access isn't allowed */ >> + up->iobase = 0; >> + } >> + break; >> + } >> +} > > WARNING: vmlinux.o(.text+0x4fdb0): Section mismatch in reference from the function jailhouse_serial_fixup() to the variable .init.data:can_use_brk_pgt > The function jailhouse_serial_fixup() references > the variable __initdata can_use_brk_pgt. > This is often because jailhouse_serial_fixup lacks a __initdata > annotation or the annotation of can_use_brk_pgt is wrong. > Yep, jailhouse_serial_fixup can become __init, I didn't see that, but you're right, thanks. I'm curious, how did you find that? "We" didn't notice yet. :-) BTW, we refers to the Jailhouse folks, but I will rewrite that. Thanks Ralf ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 2/2] x86/jailhouse: Only enable platform UARTs if available 2019-10-07 16:44 ` Ralf Ramsauer @ 2019-10-07 16:59 ` Borislav Petkov 0 siblings, 0 replies; 11+ messages in thread From: Borislav Petkov @ 2019-10-07 16:59 UTC (permalink / raw) To: Ralf Ramsauer Cc: Jan Kiszka, x86, jailhouse-dev, linux-kernel, Ingo Molnar, H . Peter Anvin On Mon, Oct 07, 2019 at 06:44:39PM +0200, Ralf Ramsauer wrote: > Yep, jailhouse_serial_fixup can become __init, I didn't see that, but > you're right, thanks. I'm curious, how did you find that? CONFIG_SECTION_MISMATCH_WARN_ONLY=y If that it off, it fails the build even: WARNING: vmlinux.o(.text+0x4fdb0): Section mismatch in reference from the function jailhouse_serial_fixup() to the variable .init.data:can_use_brk_pgt The function jailhouse_serial_fixup() references the variable __initdata can_use_brk_pgt. This is often because jailhouse_serial_fixup lacks a __initdata annotation or the annotation of can_use_brk_pgt is wrong. FATAL: modpost: Section mismatches detected. Set CONFIG_SECTION_MISMATCH_WARN_ONLY=y to allow them. make[1]: *** [scripts/Makefile.modpost:66: __modpost] Error 1 make: *** [Makefile:1074: vmlinux] Error 2 Apparently we did that with: 47490ec141b9 ("modpost: Add flag -E for making section mismatches fatal") > "We" didn't notice yet. :-) LOL. > BTW, we refers to the Jailhouse folks, but I will rewrite that. Thanks. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 11+ messages in thread
* [tip: x86/platform] x86/jailhouse: Enable platform UARTs only if available 2019-10-07 12:38 ` [PATCH v5 2/2] x86/jailhouse: Only enable platform UARTs if available Ralf Ramsauer 2019-10-07 15:20 ` Jan Kiszka 2019-10-07 16:26 ` Borislav Petkov @ 2019-10-07 18:25 ` tip-bot2 for Ralf Ramsauer 2019-10-07 18:27 ` Ingo Molnar 2 siblings, 1 reply; 11+ messages in thread From: tip-bot2 for Ralf Ramsauer @ 2019-10-07 18:25 UTC (permalink / raw) To: linux-tip-commits Cc: Ralf Ramsauer, Jan Kiszka, Borislav Petkov, H . Peter Anvin, Linus Torvalds, Peter Zijlstra, Thomas Gleixner, jailhouse-dev, Ingo Molnar, linux-kernel The following commit has been merged into the x86/platform branch of tip: Commit-ID: dac59f1eb78123c3f0e497eb9870ac550c59debb Gitweb: https://git.kernel.org/tip/dac59f1eb78123c3f0e497eb9870ac550c59debb Author: Ralf Ramsauer <ralf.ramsauer@oth-regensburg.de> AuthorDate: Mon, 07 Oct 2019 14:38:19 +02:00 Committer: Ingo Molnar <mingo@kernel.org> CommitterDate: Mon, 07 Oct 2019 17:35:56 +02:00 x86/jailhouse: Enable platform UARTs only if available ACPI tables aren't available if Linux runs as guest of the hypervisor Jailhouse. This makes the 8250 driver probe for all platform UARTs as it assumes that all platform are present in case of !ACPI. Jailhouse will stop execution of Linux guest due to port access violation. So far, these access violations could be solved by tuning the 8250.nr_uarts parameter but it has limitations: We can, e.g., only map consecutive platform UARTs to Linux, and only in the sequence 0x3f8, 0x2f8, 0x3e8, 0x2e8. Beginning from setup_data version 2, Jailhouse will place information of available platform UARTs in setup_data. This allows for selective activation of platform UARTs. This patch queries the setup_data version and activates only available UARTS. It comes with backward compatibility, and will still support older setup_data versions. In this case, Linux falls back to the old behaviour. Signed-off-by: Ralf Ramsauer <ralf.ramsauer@oth-regensburg.de> Reviewed-by: Jan Kiszka <jan.kiszka@siemens.com> Cc: Borislav Petkov <bp@alien8.de> Cc: H . Peter Anvin <hpa@zytor.com> Cc: Jan Kiszka <jan.kiszka@siemens.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: jailhouse-dev@googlegroups.com Link: https://lkml.kernel.org/r/20191007123819.161432-3-ralf.ramsauer@oth-regensburg.de Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/include/uapi/asm/bootparam.h | 3 +- arch/x86/kernel/jailhouse.c | 83 ++++++++++++++++++++++---- 2 files changed, 74 insertions(+), 12 deletions(-) diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h index 43be437..db1e24e 100644 --- a/arch/x86/include/uapi/asm/bootparam.h +++ b/arch/x86/include/uapi/asm/bootparam.h @@ -152,6 +152,9 @@ struct jailhouse_setup_data { __u8 standard_ioapic; __u8 cpu_ids[255]; } __attribute__((packed)) v1; + struct { + __u32 flags; + } __attribute__((packed)) v2; } __attribute__((packed)); /* The so-called "zeropage" */ diff --git a/arch/x86/kernel/jailhouse.c b/arch/x86/kernel/jailhouse.c index b9647ad..95550c0 100644 --- a/arch/x86/kernel/jailhouse.c +++ b/arch/x86/kernel/jailhouse.c @@ -11,6 +11,7 @@ #include <linux/acpi_pmtmr.h> #include <linux/kernel.h> #include <linux/reboot.h> +#include <linux/serial_8250.h> #include <asm/apic.h> #include <asm/cpu.h> #include <asm/hypervisor.h> @@ -23,9 +24,22 @@ static __initdata struct jailhouse_setup_data setup_data; #define SETUP_DATA_V1_LEN (sizeof(setup_data.hdr) + sizeof(setup_data.v1)) +#define SETUP_DATA_V2_LEN (SETUP_DATA_V1_LEN + sizeof(setup_data.v2)) static unsigned int precalibrated_tsc_khz; +static void jailhouse_setup_irq(unsigned int irq) +{ + struct mpc_intsrc mp_irq = { + .type = MP_INTSRC, + .irqtype = mp_INT, + .irqflag = MP_IRQPOL_ACTIVE_HIGH | MP_IRQTRIG_EDGE, + .srcbusirq = irq, + .dstirq = irq, + }; + mp_save_irq(&mp_irq); +} + static uint32_t jailhouse_cpuid_base(void) { if (boot_cpu_data.cpuid_level < 0 || @@ -79,11 +93,6 @@ static void __init jailhouse_get_smp_config(unsigned int early) .type = IOAPIC_DOMAIN_STRICT, .ops = &mp_ioapic_irqdomain_ops, }; - struct mpc_intsrc mp_irq = { - .type = MP_INTSRC, - .irqtype = mp_INT, - .irqflag = MP_IRQPOL_ACTIVE_HIGH | MP_IRQTRIG_EDGE, - }; unsigned int cpu; jailhouse_x2apic_init(); @@ -100,12 +109,12 @@ static void __init jailhouse_get_smp_config(unsigned int early) if (setup_data.v1.standard_ioapic) { mp_register_ioapic(0, 0xfec00000, gsi_top, &ioapic_cfg); - /* Register 1:1 mapping for legacy UART IRQs 3 and 4 */ - mp_irq.srcbusirq = mp_irq.dstirq = 3; - mp_save_irq(&mp_irq); - - mp_irq.srcbusirq = mp_irq.dstirq = 4; - mp_save_irq(&mp_irq); + if (IS_ENABLED(CONFIG_SERIAL_8250) && + setup_data.hdr.version < 2) { + /* Register 1:1 mapping for legacy UART IRQs 3 and 4 */ + jailhouse_setup_irq(3); + jailhouse_setup_irq(4); + } } } @@ -138,6 +147,53 @@ static int __init jailhouse_pci_arch_init(void) return 0; } +#ifdef CONFIG_SERIAL_8250 +static bool jailhouse_uart_enabled(unsigned int uart_nr) +{ + return setup_data.v2.flags & BIT(uart_nr); +} + +static void jailhouse_serial_fixup(int port, struct uart_port *up, + u32 *capabilities) +{ + static const u16 pcuart_base[] = {0x3f8, 0x2f8, 0x3e8, 0x2e8}; + unsigned int n; + + for (n = 0; n < ARRAY_SIZE(pcuart_base); n++) { + if (pcuart_base[n] != up->iobase) + continue; + + if (jailhouse_uart_enabled(n)) { + pr_info("Enabling UART%u (port 0x%lx)\n", n, + up->iobase); + jailhouse_setup_irq(up->irq); + } else { + /* Deactivate UART if access isn't allowed */ + up->iobase = 0; + } + break; + } +} + +static void jailhouse_serial_workaround(void) +{ + /* + * There are flags inside setup_data that indicate availability of + * platform UARTs since setup data version 2. + * + * In case of version 1, we don't know which UARTs belong Linux. In + * this case, unconditionally register 1:1 mapping for legacy UART IRQs + * 3 and 4. + */ + if (setup_data.hdr.version > 1) + serial8250_set_isa_configurator(jailhouse_serial_fixup); +} +#else /* !CONFIG_SERIAL_8250 */ +static inline void jailhouse_serial_workaround(void) +{ +} +#endif /* CONFIG_SERIAL_8250 */ + static void __init jailhouse_init_platform(void) { u64 pa_data = boot_params.hdr.setup_data; @@ -188,7 +244,8 @@ static void __init jailhouse_init_platform(void) if (setup_data.hdr.version == 0 || setup_data.hdr.compatible_version != JAILHOUSE_SETUP_REQUIRED_VERSION || - (setup_data.hdr.version >= 1 && header.len < SETUP_DATA_V1_LEN)) + (setup_data.hdr.version == 1 && header.len < SETUP_DATA_V1_LEN) || + (setup_data.hdr.version >= 2 && header.len < SETUP_DATA_V2_LEN)) goto unsupported; pmtmr_ioport = setup_data.v1.pm_timer_address; @@ -204,6 +261,8 @@ static void __init jailhouse_init_platform(void) * are none in a non-root cell. */ disable_acpi(); + + jailhouse_serial_workaround(); return; unsupported: ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [tip: x86/platform] x86/jailhouse: Enable platform UARTs only if available 2019-10-07 18:25 ` [tip: x86/platform] x86/jailhouse: Enable platform UARTs only " tip-bot2 for Ralf Ramsauer @ 2019-10-07 18:27 ` Ingo Molnar 0 siblings, 0 replies; 11+ messages in thread From: Ingo Molnar @ 2019-10-07 18:27 UTC (permalink / raw) To: linux-kernel Cc: linux-tip-commits, Ralf Ramsauer, Jan Kiszka, Borislav Petkov, H . Peter Anvin, Linus Torvalds, Peter Zijlstra, Thomas Gleixner, jailhouse-dev * tip-bot2 for Ralf Ramsauer <tip-bot2@linutronix.de> wrote: > The following commit has been merged into the x86/platform branch of tip: > > Commit-ID: dac59f1eb78123c3f0e497eb9870ac550c59debb > Gitweb: https://git.kernel.org/tip/dac59f1eb78123c3f0e497eb9870ac550c59debb > Author: Ralf Ramsauer <ralf.ramsauer@oth-regensburg.de> > AuthorDate: Mon, 07 Oct 2019 14:38:19 +02:00 > Committer: Ingo Molnar <mingo@kernel.org> > CommitterDate: Mon, 07 Oct 2019 17:35:56 +02:00 > > x86/jailhouse: Enable platform UARTs only if available Never mind this commit notification - I pulled the two commits for the time being, until Boris's feedback is addressed. Thanks, Ingo ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-10-07 18:27 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-07 12:38 [PATCH v5 0/2] x86/jailhouse: improve probing of platform UARTs Ralf Ramsauer 2019-10-07 12:38 ` [PATCH v5 1/2] x86/jailhouse: improve setup data version comparison Ralf Ramsauer 2019-10-07 16:25 ` Borislav Petkov 2019-10-07 18:25 ` [tip: x86/platform] x86/jailhouse: Improve " tip-bot2 for Ralf Ramsauer 2019-10-07 12:38 ` [PATCH v5 2/2] x86/jailhouse: Only enable platform UARTs if available Ralf Ramsauer 2019-10-07 15:20 ` Jan Kiszka 2019-10-07 16:26 ` Borislav Petkov 2019-10-07 16:44 ` Ralf Ramsauer 2019-10-07 16:59 ` Borislav Petkov 2019-10-07 18:25 ` [tip: x86/platform] x86/jailhouse: Enable platform UARTs only " tip-bot2 for Ralf Ramsauer 2019-10-07 18:27 ` Ingo Molnar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).