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 -- 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/c106ddb4-809e-0ae9-df02-2e5b22f23b03%40oth-regensburg.de.
