Re: amd64 serial console changes, part 2

2022-07-13 Thread Mark Kettenis
> Date: Tue, 12 Jul 2022 20:03:15 +0200
> From: Denis Fondras 
> 
> Le Wed, Jul 06, 2022 at 10:45:39PM +0200, Mark Kettenis a écrit :
> > Now that the kernel supports the extended BOOTARG_CONSDEV struct and
> > snaps with that change are out there, here is the diff that changes
> > the amd64 bootloaders to switch to the extended struct and provide the
> > parameters necessary for using the non-standard UART on the AMD Ryzen
> > Embedded V1000 SoCs.
> > 
> > It would be good if someone can confirm this works on something like
> > an APU.
> > 
> 
> I don't have any other EFI appliance to test but it reads fine, applies and
> builds OK.
> 
> Anyway, I could not make it work on the AMD Ryzen Embedded V1000. I
> might be missing a step here. I built a kernel with the diff
> applied, built the ramdrive and tried to boot it but it still
> reboots when in ELFNAME().

You'll need an image with an updated bootloader; check that the
version number of the bootloader is at least 3.61.

The diff is committed now, so the easiest thing is to just wait for a
fresh snapshot.

Cheers,

Mark



Re: amd64 serial console changes, part 2

2022-07-12 Thread Denis Fondras
Le Wed, Jul 06, 2022 at 10:45:39PM +0200, Mark Kettenis a écrit :
> Now that the kernel supports the extended BOOTARG_CONSDEV struct and
> snaps with that change are out there, here is the diff that changes
> the amd64 bootloaders to switch to the extended struct and provide the
> parameters necessary for using the non-standard UART on the AMD Ryzen
> Embedded V1000 SoCs.
> 
> It would be good if someone can confirm this works on something like
> an APU.
> 

I don't have any other EFI appliance to test but it reads fine, applies and
builds OK.

Anyway, I could not make it work on the AMD Ryzen Embedded V1000. I might be
missing a step here. I built a kernel with the diff applied, built the ramdrive
and tried to boot it but it still reboots when in ELFNAME().

> ok?
> 



> 
> Index: stand/boot/conf.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/stand/boot/conf.c,v
> retrieving revision 1.53
> diff -u -p -r1.53 conf.c
> --- stand/boot/conf.c 9 Dec 2020 18:10:17 -   1.53
> +++ stand/boot/conf.c 6 Jul 2022 20:02:13 -
> @@ -41,7 +41,7 @@
>  #include 
>  #include 
>  
> -const char version[] = "3.53";
> +const char version[] = "3.54";
>  int  debug = 1;
>  
>  
> Index: stand/cdboot/conf.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/stand/cdboot/conf.c,v
> retrieving revision 1.47
> diff -u -p -r1.47 conf.c
> --- stand/cdboot/conf.c   9 Dec 2020 18:10:18 -   1.47
> +++ stand/cdboot/conf.c   6 Jul 2022 20:02:13 -
> @@ -42,7 +42,7 @@
>  #include 
>  #include 
>  
> -const char version[] = "3.53";
> +const char version[] = "3.54";
>  int  debug = 1;
>  
>  
> Index: stand/efiboot/conf.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/conf.c,v
> retrieving revision 1.37
> diff -u -p -r1.37 conf.c
> --- stand/efiboot/conf.c  20 Jun 2022 02:22:05 -  1.37
> +++ stand/efiboot/conf.c  6 Jul 2022 20:02:13 -
> @@ -40,7 +40,7 @@
>  #include "efidev.h"
>  #include "efipxe.h"
>  
> -const char version[] = "3.60";
> +const char version[] = "3.61";
>  
>  #ifdef EFI_DEBUG
>  int  debug = 0;
> Index: stand/efiboot/efiboot.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/efiboot.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 efiboot.c
> --- stand/efiboot/efiboot.c   20 Jun 2022 02:22:05 -  1.39
> +++ stand/efiboot/efiboot.c   6 Jul 2022 20:02:13 -
> @@ -938,6 +938,70 @@ efi_makebootargs(void)
>   addbootarg(BOOTARG_EFIINFO, sizeof(bios_efiinfo), _efiinfo);
>  }
>  
> +/* Vendor device path used to indicate the mmio UART on AMD SoCs. */
> +#define AMDSOC_DEVPATH \
> + { 0xe76fd4e9, 0x0a30, 0x4ca9, \
> + { 0x95, 0x40, 0xd7, 0x99, 0x53, 0x4c, 0xc4, 0xff } }
> +
> +void
> +efi_setconsdev(void)
> +{
> + bios_consdev_t cd;
> + EFI_STATUS status;
> + UINT8 data[128];
> + UINTN size = sizeof(data);
> + EFI_DEVICE_PATH *dp = (void *)data;
> + VENDOR_DEVICE_PATH *vdp;
> + UART_DEVICE_PATH *udp;
> + EFI_GUID global = EFI_GLOBAL_VARIABLE;
> + EFI_GUID amdsoc = AMDSOC_DEVPATH;
> +
> + memset(, 0, sizeof(cd));
> + cd.consdev = cn_tab->cn_dev;
> + cd.conspeed = com_speed;
> + cd.consaddr = com_addr;
> +
> + /*
> +  * If the ConOut variable indicates we're using a serial
> +  * console, use it to determine the baud rate.
> +  */
> + status = RS->GetVariable(L"ConOut", , NULL, , );
> + if (status == EFI_SUCCESS) {
> + for (dp = (void *)data; !IsDevicePathEnd(dp);
> +  dp = NextDevicePathNode(dp)) {
> + /*
> +  * AMD Ryzen Embedded V1000 SoCs integrate a
> +  * Synopsys DesignWare UART that is not
> +  * compatible with the traditional 8250 UART
> +  * found on the IBM PC.  Pass the magic
> +  * parameters to the kernel to make this UART
> +  * work.
> +  */
> + if (DevicePathType(dp) == HARDWARE_DEVICE_PATH &&
> + DevicePathSubType(dp) == HW_VENDOR_DP) {
> + vdp = (VENDOR_DEVICE_PATH *)dp;
> + if (efi_guidcmp(>Guid, ) == 0) {
> + cd.consdev = makedev(8, 4);
> + cd.consaddr = *(uint64_t *)(vdp + 1);
> + cd.consfreq = 4800;
> + cd.flags = BCD_MMIO;
> + cd.reg_width = 4;
> + cd.reg_shift = 2;
> + }
> + }
> +
> + if (DevicePathType(dp) == MESSAGING_DEVICE_PATH &&

Re: amd64 serial console changes, part 2

2022-07-08 Thread Hrvoje Popovski
On 6.7.2022. 22:45, Mark Kettenis wrote:
> Now that the kernel supports the extended BOOTARG_CONSDEV struct and
> snaps with that change are out there, here is the diff that changes
> the amd64 bootloaders to switch to the extended struct and provide the
> parameters necessary for using the non-standard UART on the AMD Ryzen
> Embedded V1000 SoCs.
> 
> It would be good if someone can confirm this works on something like
> an APU.

Hi,

I've tested this on Dell r620 with serial console and on supermicro box
with ipmi serial and both works normally. This time with everything that
needs to be compiled :)


dell
com1 at acpi0 COMA addr 0x2f8/0x8 irq 3: ns16550a, 16 byte fifo
com1: console
com0 at acpi0 COMB addr 0x3f8/0x8 irq 4: ns16550a, 16 byte fifo


supermicro
com0 at acpi0 UAR1 addr 0x3f8/0x8 irq 4: ns16550a, 16 byte fifo
com1 at acpi0 UAR2 addr 0x2f8/0x8 irq 3: ns16550a, 16 byte fifo
com1: console



amd64 serial console changes, part 2

2022-07-06 Thread Mark Kettenis
Now that the kernel supports the extended BOOTARG_CONSDEV struct and
snaps with that change are out there, here is the diff that changes
the amd64 bootloaders to switch to the extended struct and provide the
parameters necessary for using the non-standard UART on the AMD Ryzen
Embedded V1000 SoCs.

It would be good if someone can confirm this works on something like
an APU.

ok?


Index: stand/boot/conf.c
===
RCS file: /cvs/src/sys/arch/amd64/stand/boot/conf.c,v
retrieving revision 1.53
diff -u -p -r1.53 conf.c
--- stand/boot/conf.c   9 Dec 2020 18:10:17 -   1.53
+++ stand/boot/conf.c   6 Jul 2022 20:02:13 -
@@ -41,7 +41,7 @@
 #include 
 #include 
 
-const char version[] = "3.53";
+const char version[] = "3.54";
 intdebug = 1;
 
 
Index: stand/cdboot/conf.c
===
RCS file: /cvs/src/sys/arch/amd64/stand/cdboot/conf.c,v
retrieving revision 1.47
diff -u -p -r1.47 conf.c
--- stand/cdboot/conf.c 9 Dec 2020 18:10:18 -   1.47
+++ stand/cdboot/conf.c 6 Jul 2022 20:02:13 -
@@ -42,7 +42,7 @@
 #include 
 #include 
 
-const char version[] = "3.53";
+const char version[] = "3.54";
 intdebug = 1;
 
 
Index: stand/efiboot/conf.c
===
RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/conf.c,v
retrieving revision 1.37
diff -u -p -r1.37 conf.c
--- stand/efiboot/conf.c20 Jun 2022 02:22:05 -  1.37
+++ stand/efiboot/conf.c6 Jul 2022 20:02:13 -
@@ -40,7 +40,7 @@
 #include "efidev.h"
 #include "efipxe.h"
 
-const char version[] = "3.60";
+const char version[] = "3.61";
 
 #ifdef EFI_DEBUG
 intdebug = 0;
Index: stand/efiboot/efiboot.c
===
RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/efiboot.c,v
retrieving revision 1.39
diff -u -p -r1.39 efiboot.c
--- stand/efiboot/efiboot.c 20 Jun 2022 02:22:05 -  1.39
+++ stand/efiboot/efiboot.c 6 Jul 2022 20:02:13 -
@@ -938,6 +938,70 @@ efi_makebootargs(void)
addbootarg(BOOTARG_EFIINFO, sizeof(bios_efiinfo), _efiinfo);
 }
 
+/* Vendor device path used to indicate the mmio UART on AMD SoCs. */
+#define AMDSOC_DEVPATH \
+   { 0xe76fd4e9, 0x0a30, 0x4ca9, \
+   { 0x95, 0x40, 0xd7, 0x99, 0x53, 0x4c, 0xc4, 0xff } }
+
+void
+efi_setconsdev(void)
+{
+   bios_consdev_t cd;
+   EFI_STATUS status;
+   UINT8 data[128];
+   UINTN size = sizeof(data);
+   EFI_DEVICE_PATH *dp = (void *)data;
+   VENDOR_DEVICE_PATH *vdp;
+   UART_DEVICE_PATH *udp;
+   EFI_GUID global = EFI_GLOBAL_VARIABLE;
+   EFI_GUID amdsoc = AMDSOC_DEVPATH;
+
+   memset(, 0, sizeof(cd));
+   cd.consdev = cn_tab->cn_dev;
+   cd.conspeed = com_speed;
+   cd.consaddr = com_addr;
+
+   /*
+* If the ConOut variable indicates we're using a serial
+* console, use it to determine the baud rate.
+*/
+   status = RS->GetVariable(L"ConOut", , NULL, , );
+   if (status == EFI_SUCCESS) {
+   for (dp = (void *)data; !IsDevicePathEnd(dp);
+dp = NextDevicePathNode(dp)) {
+   /*
+* AMD Ryzen Embedded V1000 SoCs integrate a
+* Synopsys DesignWare UART that is not
+* compatible with the traditional 8250 UART
+* found on the IBM PC.  Pass the magic
+* parameters to the kernel to make this UART
+* work.
+*/
+   if (DevicePathType(dp) == HARDWARE_DEVICE_PATH &&
+   DevicePathSubType(dp) == HW_VENDOR_DP) {
+   vdp = (VENDOR_DEVICE_PATH *)dp;
+   if (efi_guidcmp(>Guid, ) == 0) {
+   cd.consdev = makedev(8, 4);
+   cd.consaddr = *(uint64_t *)(vdp + 1);
+   cd.consfreq = 4800;
+   cd.flags = BCD_MMIO;
+   cd.reg_width = 4;
+   cd.reg_shift = 2;
+   }
+   }
+
+   if (DevicePathType(dp) == MESSAGING_DEVICE_PATH &&
+   DevicePathSubType(dp) == MSG_UART_DP) {
+   udp = (UART_DEVICE_PATH *)dp;
+   if (cd.conspeed == -1)
+   cd.conspeed = udp->BaudRate;
+   }
+   }
+   }
+
+   addbootarg(BOOTARG_CONSDEV, sizeof(cd), );
+}
+
 void
 _rtt(void)
 {
Index: stand/efiboot/efiboot.h
===
RCS file: