Re: [PATCH v5 2/2] x86/jailhouse: Only enable platform UARTs if available
From: Jan Kiszka
Date: Mon Oct 07 2019 - 11:22:13 EST
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@xxxxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxx>
Thanks,
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux