diff mbox series

[kvm-unit-tests,v6,12/32] arm64: Add support for discovering the UART through ACPI

Message ID 20230530160924.82158-13-nikos.nikoleris@arm.com (mailing list archive)
State New, archived
Headers show
Series EFI and ACPI support for arm64 | expand

Commit Message

Nikos Nikoleris May 30, 2023, 4:09 p.m. UTC 
In systems with ACPI support and when a DT is not provided, we can use
the SPCR to discover the serial port address range. This change
implements this but retains the default behavior; we check if a valid
DT is provided, if not, we try to discover the UART using ACPI.

Signed-off-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Ricardo Koller <ricarkol@google.com>
Reviewed-by: Shaoqin Huang <shahuang@redhat.com>
---
 lib/acpi.h   | 25 +++++++++++++++++++++++++
 lib/arm/io.c | 34 +++++++++++++++++++++++++++++-----
 2 files changed, 54 insertions(+), 5 deletions(-)

Comments

Nadav Amit June 8, 2023, 5:18 p.m. UTC | #1 
On May 30, 2023, at 9:09 AM, Nikos Nikoleris <nikos.nikoleris@arm.com> wrote:

> 
> +static void uart0_init_acpi(void)
> +{
> +	struct spcr_descriptor *spcr = find_acpi_table_addr(SPCR_SIGNATURE);
> +
> +	assert_msg(spcr, "Unable to find ACPI SPCR");
> +	uart0_base = ioremap(spcr->serial_port.address, spcr->serial_port.bit_width);
> +}

Is it possible as a fallback, is SPCR is not available, to UART_EARLY_BASE as
address and bit_width as bit-width?

I would appreciate it, since it would help my setup.
Nadav Amit June 8, 2023, 5:24 p.m. UTC | #2 
> On Jun 8, 2023, at 10:18 AM, Nadav Amit <nadav.amit@gmail.com> wrote:
> 
> 
> On May 30, 2023, at 9:09 AM, Nikos Nikoleris <nikos.nikoleris@arm.com> wrote:
> 
>> 
>> +static void uart0_init_acpi(void)
>> +{
>> + struct spcr_descriptor *spcr = find_acpi_table_addr(SPCR_SIGNATURE);
>> +
>> + assert_msg(spcr, "Unable to find ACPI SPCR");
>> + uart0_base = ioremap(spcr->serial_port.address, spcr->serial_port.bit_width);
>> +}
> 
> Is it possible as a fallback, is SPCR is not available, to UART_EARLY_BASE as
> address and bit_width as bit-width?
> 
> I would appreciate it, since it would help my setup.
> 

Ugh - typo, 8 as bit-width for the fallback (ioremap with these parameters to
make my request clear).
Andrew Jones June 9, 2023, 7:21 a.m. UTC | #3 
On Thu, Jun 08, 2023 at 10:24:11AM -0700, Nadav Amit wrote:
> 
> 
> > On Jun 8, 2023, at 10:18 AM, Nadav Amit <nadav.amit@gmail.com> wrote:
> > 
> > 
> > On May 30, 2023, at 9:09 AM, Nikos Nikoleris <nikos.nikoleris@arm.com> wrote:
> > 
> >> 
> >> +static void uart0_init_acpi(void)
> >> +{
> >> + struct spcr_descriptor *spcr = find_acpi_table_addr(SPCR_SIGNATURE);
> >> +
> >> + assert_msg(spcr, "Unable to find ACPI SPCR");
> >> + uart0_base = ioremap(spcr->serial_port.address, spcr->serial_port.bit_width);
> >> +}
> > 
> > Is it possible as a fallback, is SPCR is not available, to UART_EARLY_BASE as
> > address and bit_width as bit-width?
> > 
> > I would appreciate it, since it would help my setup.
> > 
> 
> Ugh - typo, 8 as bit-width for the fallback (ioremap with these parameters to
> make my request clear).
>

That sounds reasonable to me. Nikos, can you send a fixup! patch? I'll
squash it in.

Thanks,
drew
Nikos Nikoleris June 9, 2023, 2:06 p.m. UTC | #4 
On 09/06/2023 08:21, Andrew Jones wrote:
> On Thu, Jun 08, 2023 at 10:24:11AM -0700, Nadav Amit wrote:
>>
>>
>>> On Jun 8, 2023, at 10:18 AM, Nadav Amit <nadav.amit@gmail.com> wrote:
>>>
>>>
>>> On May 30, 2023, at 9:09 AM, Nikos Nikoleris <nikos.nikoleris@arm.com> wrote:
>>>
>>>>
>>>> +static void uart0_init_acpi(void)
>>>> +{
>>>> + struct spcr_descriptor *spcr = find_acpi_table_addr(SPCR_SIGNATURE);
>>>> +
>>>> + assert_msg(spcr, "Unable to find ACPI SPCR");
>>>> + uart0_base = ioremap(spcr->serial_port.address, spcr->serial_port.bit_width);
>>>> +}
>>>
>>> Is it possible as a fallback, is SPCR is not available, to UART_EARLY_BASE as
>>> address and bit_width as bit-width?
>>>
>>> I would appreciate it, since it would help my setup.
>>>
>>
>> Ugh - typo, 8 as bit-width for the fallback (ioremap with these parameters to
>> make my request clear).
>>
> 
> That sounds reasonable to me. Nikos, can you send a fixup! patch? I'll
> squash it in.
> 

I am not against this idea, but it's not something that we do when we 
setup the uart through FDT. Should ACPI behave differently? Is this 
really a fixup? Either ACPI will setup things differently or we'll 
change the FDT and ACPI path.

Thanks,

Nikos
Andrew Jones June 9, 2023, 2:31 p.m. UTC | #5 
On Fri, Jun 09, 2023 at 03:06:36PM +0100, Nikos Nikoleris wrote:
> On 09/06/2023 08:21, Andrew Jones wrote:
> > On Thu, Jun 08, 2023 at 10:24:11AM -0700, Nadav Amit wrote:
> > > 
> > > 
> > > > On Jun 8, 2023, at 10:18 AM, Nadav Amit <nadav.amit@gmail.com> wrote:
> > > > 
> > > > 
> > > > On May 30, 2023, at 9:09 AM, Nikos Nikoleris <nikos.nikoleris@arm.com> wrote:
> > > > 
> > > > > 
> > > > > +static void uart0_init_acpi(void)
> > > > > +{
> > > > > + struct spcr_descriptor *spcr = find_acpi_table_addr(SPCR_SIGNATURE);
> > > > > +
> > > > > + assert_msg(spcr, "Unable to find ACPI SPCR");
> > > > > + uart0_base = ioremap(spcr->serial_port.address, spcr->serial_port.bit_width);
> > > > > +}
> > > > 
> > > > Is it possible as a fallback, is SPCR is not available, to UART_EARLY_BASE as
> > > > address and bit_width as bit-width?
> > > > 
> > > > I would appreciate it, since it would help my setup.
> > > > 
> > > 
> > > Ugh - typo, 8 as bit-width for the fallback (ioremap with these parameters to
> > > make my request clear).
> > > 
> > 
> > That sounds reasonable to me. Nikos, can you send a fixup! patch? I'll
> > squash it in.
> > 
> 
> I am not against this idea, but it's not something that we do when we setup
> the uart through FDT. Should ACPI behave differently? Is this really a
> fixup? Either ACPI will setup things differently or we'll change the FDT and
> ACPI path.

Yeah, you're right. It's not really a fixup and I forgot that we abort
when the DT doesn't have a uart node too. Let's leave this as is for now.
We can do a follow patch which adds a config that says "use the early
uart and don't bother looking for another" which we'd apply to both ACPI
and DT.

Thanks,
drew
Nadav Amit June 10, 2023, 1:13 a.m. UTC | #6 
> On Jun 9, 2023, at 7:31 AM, Andrew Jones <andrew.jones@linux.dev> wrote:
> 
> On Fri, Jun 09, 2023 at 03:06:36PM +0100, Nikos Nikoleris wrote:
>> On 09/06/2023 08:21, Andrew Jones wrote:
>>> On Thu, Jun 08, 2023 at 10:24:11AM -0700, Nadav Amit wrote:
>>>> 
>>>> 
>>>>> On Jun 8, 2023, at 10:18 AM, Nadav Amit <nadav.amit@gmail.com> wrote:
>>>>> 
>>>>> 
>>>>> On May 30, 2023, at 9:09 AM, Nikos Nikoleris <nikos.nikoleris@arm.com> wrote:
>>>>> 
>>>>>> 
>>>>>> +static void uart0_init_acpi(void)
>>>>>> +{
>>>>>> + struct spcr_descriptor *spcr = find_acpi_table_addr(SPCR_SIGNATURE);
>>>>>> +
>>>>>> + assert_msg(spcr, "Unable to find ACPI SPCR");
>>>>>> + uart0_base = ioremap(spcr->serial_port.address, spcr->serial_port.bit_width);
>>>>>> +}
>>>>> 
>>>>> Is it possible as a fallback, is SPCR is not available, to UART_EARLY_BASE as
>>>>> address and bit_width as bit-width?
>>>>> 
>>>>> I would appreciate it, since it would help my setup.
>>>>> 
>>>> 
>>>> Ugh - typo, 8 as bit-width for the fallback (ioremap with these parameters to
>>>> make my request clear).
>>>> 
>>> 
>>> That sounds reasonable to me. Nikos, can you send a fixup! patch? I'll
>>> squash it in.
>>> 
>> 
>> I am not against this idea, but it's not something that we do when we setup
>> the uart through FDT. Should ACPI behave differently? Is this really a
>> fixup? Either ACPI will setup things differently or we'll change the FDT and
>> ACPI path.
> 
> Yeah, you're right. It's not really a fixup and I forgot that we abort
> when the DT doesn't have a uart node too. Let's leave this as is for now.
> We can do a follow patch which adds a config that says "use the early
> uart and don't bother looking for another" which we'd apply to both ACPI
> and DT.

Ok. I’ll add some option later to force it, perhaps if UART_EARLY_BASE is
different than the default.
diff mbox series

Patch

diff --git a/lib/acpi.h b/lib/acpi.h
index 1ba4999e..8c4598e8 100644
--- a/lib/acpi.h
+++ b/lib/acpi.h
@@ -17,6 +17,7 @@ 
 #define XSDT_SIGNATURE ACPI_SIGNATURE('X','S','D','T')
 #define FACP_SIGNATURE ACPI_SIGNATURE('F','A','C','P')
 #define FACS_SIGNATURE ACPI_SIGNATURE('F','A','C','S')
+#define SPCR_SIGNATURE ACPI_SIGNATURE('S','P','C','R')
 
 #define ACPI_SIGNATURE_8BYTE(c1, c2, c3, c4, c5, c6, c7, c8) \
 	(((uint64_t)(ACPI_SIGNATURE(c1, c2, c3, c4))) |	     \
@@ -145,6 +146,30 @@  struct acpi_table_facs_rev1 {
 	u8 reserved3[40];	/* Reserved - must be zero */
 };
 
+struct spcr_descriptor {
+	ACPI_TABLE_HEADER_DEF	/* ACPI common table header */
+	u8 interface_type;	/* 0=full 16550, 1=subset of 16550 */
+	u8 reserved[3];
+	struct acpi_generic_address serial_port;
+	u8 interrupt_type;
+	u8 pc_interrupt;
+	u32 interrupt;
+	u8 baud_rate;
+	u8 parity;
+	u8 stop_bits;
+	u8 flow_control;
+	u8 terminal_type;
+	u8 reserved1;
+	u16 pci_device_id;
+	u16 pci_vendor_id;
+	u8 pci_bus;
+	u8 pci_device;
+	u8 pci_function;
+	u32 pci_flags;
+	u8 pci_segment;
+	u32 reserved2;
+};
+
 /* Reset to default packing */
 #pragma pack()
 
diff --git a/lib/arm/io.c b/lib/arm/io.c
index 343e1082..19f93490 100644
--- a/lib/arm/io.c
+++ b/lib/arm/io.c
@@ -29,7 +29,7 @@  static struct spinlock uart_lock;
 #define UART_EARLY_BASE (u8 *)(unsigned long)CONFIG_UART_EARLY_BASE
 static volatile u8 *uart0_base = UART_EARLY_BASE;
 
-static void uart0_init(void)
+static void uart0_init_fdt(void)
 {
 	/*
 	 * kvm-unit-tests uses the uart only for output. Both uart models have
@@ -65,17 +65,41 @@  static void uart0_init(void)
 	}
 
 	uart0_base = ioremap(base.addr, base.size);
+}
+
+#ifdef CONFIG_EFI
+
+#include <acpi.h>
+
+static void uart0_init_acpi(void)
+{
+	struct spcr_descriptor *spcr = find_acpi_table_addr(SPCR_SIGNATURE);
+
+	assert_msg(spcr, "Unable to find ACPI SPCR");
+	uart0_base = ioremap(spcr->serial_port.address, spcr->serial_port.bit_width);
+}
+#else
+
+static void uart0_init_acpi(void)
+{
+	assert_msg(false, "ACPI not available");
+}
+
+#endif
+
+void io_init(void)
+{
+	if (dt_available())
+		uart0_init_fdt();
+	else
+		uart0_init_acpi();
 
 	if (uart0_base != UART_EARLY_BASE) {
 		printf("WARNING: early print support may not work. "
 		       "Found uart at %p, but early base is %p.\n",
 			uart0_base, UART_EARLY_BASE);
 	}
-}
 
-void io_init(void)
-{
-	uart0_init();
 	chr_testdev_init();
 }