On August 19, 2019 10:22:07 AM PDT, Ralf Ramsauer <[email protected]> wrote: >On 8/14/19 12:53 AM, [email protected] wrote: >> On August 13, 2019 4:04:34 AM PDT, Jan Kiszka ><[email protected]> wrote: >>> On 13.08.19 11:32, [email protected] wrote: >>>> On August 12, 2019 4:06:50 AM PDT, Ralf Ramsauer >>> <[email protected]> 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 <[email protected]> >>>>> --- >>>>> arch/x86/include/uapi/asm/bootparam.h | 3 ++ >>>>> arch/x86/kernel/jailhouse.c | 75 >>> ++++++++++++++++++++++++--- >>>>> 2 files changed, 72 insertions(+), 6 deletions(-) >>>>> >>>>> diff --git a/arch/x86/include/uapi/asm/bootparam.h >>>>> b/arch/x86/include/uapi/asm/bootparam.h >>>>> index 6163b1afa7b3..2244c493c3c5 100644 >>>>> --- a/arch/x86/include/uapi/asm/bootparam.h >>>>> +++ b/arch/x86/include/uapi/asm/bootparam.h >>>>> @@ -150,6 +150,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 e5ac35efc4b3..1c75de1496f3 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> >>>>> @@ -20,8 +21,13 @@ >>>>> #include <asm/reboot.h> >>>>> #include <asm/setup.h> >>>>> >>>>> +#define SETUP_DATA_FLAGS_PERMIT_PCUART(n) (1 << (n)) >>>>> +#define SETUP_DATA_FLAGS_HAS_PCUART(flags, n) \ >>>>> + !!(flags & SETUP_DATA_FLAGS_PERMIT_PCUART(n)) >>>>> + >>>>> 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; >>>>> >>>>> @@ -78,11 +84,13 @@ static void __init >>>>> jailhouse_get_smp_config(unsigned int early) >>>>> .type = IOAPIC_DOMAIN_STRICT, >>>>> .ops = &mp_ioapic_irqdomain_ops, >>>>> }; >>>>> +#ifdef CONFIG_SERIAL_8250 >>>>> struct mpc_intsrc mp_irq = { >>>>> .type = MP_INTSRC, >>>>> .irqtype = mp_INT, >>>>> .irqflag = MP_IRQPOL_ACTIVE_HIGH | MP_IRQTRIG_EDGE, >>>>> }; >>>>> +#endif >>>>> unsigned int cpu; >>>>> >>>>> jailhouse_x2apic_init(); >>>>> @@ -99,12 +107,16 @@ 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); >>>>> +#ifdef CONFIG_SERIAL_8250 >>>>> + if (setup_data.hdr.version < 2) { >>>>> + /* 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); >>>>> + mp_irq.srcbusirq = mp_irq.dstirq = 4; >>>>> + mp_save_irq(&mp_irq); >>>>> + } >>>>> +#endif >>>>> } >>>>> } >>>>> >>>>> @@ -137,6 +149,42 @@ static int __init >jailhouse_pci_arch_init(void) >>>>> return 0; >>>>> } >>>>> >>>>> +#ifdef CONFIG_SERIAL_8250 >>>>> +static const u16 pcuart_base[] = { >>>>> + 0x3f8, >>>>> + 0x2f8, >>>>> + 0x3e8, >>>>> + 0x2e8, >>>>> +}; >>>>> + >>>>> +static void jailhouse_serial_fixup(int port, struct uart_port >*up, >>>>> + u32 *capabilites) >>>>> +{ >>>>> + struct mpc_intsrc mp_irq = { >>>>> + .type = MP_INTSRC, >>>>> + .irqtype = mp_INT, >>>>> + .irqflag = MP_IRQPOL_ACTIVE_HIGH | MP_IRQTRIG_EDGE, >>>>> + }; >>>>> + unsigned int n; >>>>> + >>>>> + for (n = 0; n < ARRAY_SIZE(pcuart_base); n++) { >>>>> + if (pcuart_base[n] != up->iobase) >>>>> + continue; >>>>> + >>>>> + if (SETUP_DATA_FLAGS_HAS_PCUART(setup_data.v2.flags, n)) { >>>>> + pr_info("Enabling UART%u (port 0x%lx)\n", n, >>>>> + up->iobase); >>>>> + mp_irq.srcbusirq = mp_irq.dstirq = up->irq; >>>>> + mp_save_irq(&mp_irq); >>>>> + } else { >>>>> + /* Deactivate UART if access isn't allowed */ >>>>> + up->iobase = 0; >>>>> + } >>>>> + break; >>>>> + } >>>>> +} >>>>> +#endif >>>>> + >>>>> static void __init jailhouse_init_platform(void) >>>>> { >>>>> u64 pa_data = boot_params.hdr.setup_data; >>>>> @@ -186,7 +234,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; >>>>> @@ -202,6 +251,20 @@ static void __init >>> jailhouse_init_platform(void) >>>>> * are none in a non-root cell. >>>>> */ >>>>> disable_acpi(); >>>>> + >>>>> +#ifdef CONFIG_SERIAL_8250 >>>>> + /* >>>>> + * 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); >>>>> +#endif >>>>> + >>>>> return; >>>>> >>>>> unsupported: >>>> >>>> Or you could, you know, pass a data structure that already does >>> this... it's called DSDT. >>>> >>> >>> At least by the time the boot process for Linux under Jailhouse was >>> designed >>> (~2015), ACPI was not able to express the minimal hardware we are >>> exposing. So >>> we went for "CONFIG_ACPI disabled", and that was rather simple. >>> >>> There are some new knobs now to get rid of legacy platform >components. >>> But, >>> e.g., is it ACPI-compliant to expose a PM_TMR block, but nothing >else? >>> How would >>> you communicate pre-calibrated TSC and APIC frequencies? >>> >>> Thanks, >>> Jan >> >> If ACPI cannot represent something important, I'd like to bring that >to the attention of the ACPI committee so we can standardize whatever >is missing. > >In an offlist discussion, Jan and I agreed that DSDTs should definitely >be considered and evaluated in the long run, but for the moment - and >for what I actually want to achieve with this patch - this should do >the >job. > >I will send a v3 and address Jan's remarks on patch 1/2. > >Thanks > Ralf
The request still remains, though: *please* contact me if there are things we need to be able to express in ACPI and can't. -- Sent from my Android device with K-9 Mail. Please excuse my brevity. -- You received this message because you are subscribed to the Google Groups "Jailhouse" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/5F992CC8-90A5-46B4-9E78-72F9CA82232F%40zytor.com.
