[PATCH 0/8] PALcode fixes required to run NetBSD/alpha.

[PATCH 0/8] PALcode fixes required to run NetBSD/alpha.

[Jason Thorpe]

Included here are a set of patches that fix issues in qemu-palcode needed
to run NetBSD/alpha under Qemu.  Some fix garden-variety bugs, some fix
deviations from the architecture specification or behavior of the SRM
console on real Alpha hardware.

Two of the changes (patch 6 and patch 7) also require other fixes in
Qemu itself, which will be submitted separately.  However, the changes
are fully compatible with existing Qemu alpha VMs because Linux does
not use the the SRM PCI interrupt mapping information (it has its own
tables for the system variations it supports) or the Console Terminal
Block in the HWRPB.

Jason Thorpe (8):
  Make qemu-palcode build environment standalone. NFC.
  Fix delivery of unaligned access exceptions.
  Fix initialization of the hwrpb.hwrpb.cpuid field.
  Make some PCI macros available to other files.  NFC.
  Fix incorrect initialization of PCI BARs.
  Provide interrupt mapping information in PCI config registers.
  Provide a Console Terminal Block in the HWRPB.
  Fixes for seconday CPU start-up.

 hwrpb.h       | 54 +++++++++++++++++++++++++++++++
 init.c        | 88 +++++++++++++++++++++++++++++++++++++++------------
 memcpy.c      |  2 +-
 memset.c      |  2 +-
 pal.S         | 15 ++++++---
 pci.c         | 31 +++++++++++++-----
 pci.h         |  5 +++
 printf.c      |  4 +--
 protos.h      | 30 +++++++++++++++---
 sys-clipper.h | 27 ++++++++++++++++
 vgaio.c       |  2 ++
 11 files changed, 218 insertions(+), 42 deletions(-)

-- 
2.30.2

[PATCH 1/8] Make qemu-palcode build environment standalone. NFC.

[Jason Thorpe]

Don't include system headers.  Instead, provide standalone definitions
and declarations of types needed and functions used by the PALcode that
are compatible with the standard Alpha / GCC ABI.

Signed-off-by: Jason Thorpe <thorpej@me.com>
---
 init.c   |  2 --
 memcpy.c |  2 +-
 memset.c |  2 +-
 printf.c |  4 +---
 protos.h | 30 +++++++++++++++++++++++++-----
 5 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/init.c b/init.c
index b53bab6..429a9ad 100644
--- a/init.c
+++ b/init.c
@@ -18,8 +18,6 @@
    along with this program; see the file COPYING.  If not see
    <http://www.gnu.org/licenses/>.  */
 
-#include <string.h>
-#include <stddef.h>
 #include "hwrpb.h"
 #include "osf.h"
 #include "ioport.h"
diff --git a/memcpy.c b/memcpy.c
index b6bbb74..9e1e913 100644
--- a/memcpy.c
+++ b/memcpy.c
@@ -8,7 +8,7 @@
  * This is a reasonably optimized memcpy() routine.
  */
 
-#include <string.h>
+#include "protos.h"
 
 /*
  * Note that the C code is written to be optimized into good assembly. However,
diff --git a/memset.c b/memset.c
index e8481dc..f9b0a6d 100644
--- a/memset.c
+++ b/memset.c
@@ -19,7 +19,7 @@
    <http://www.gnu.org/licenses/>.  */
 
 
-#include <string.h>
+#include "protos.h"
 
 void *memset(void *optr, int ival, unsigned long size)
 {
diff --git a/printf.c b/printf.c
index 469b82c..0e1e128 100644
--- a/printf.c
+++ b/printf.c
@@ -18,10 +18,8 @@
    along with this program; see the file COPYING.  If not see
    <http://www.gnu.org/licenses/>.  */
 
-#include <stdarg.h>
-#include <stdbool.h>
-#include <string.h>
 #include "console.h"
+#include "protos.h"
 
 static int print_buf_pad(char *buf, int buflen, char *p, int width, int pad)
 {
diff --git a/protos.h b/protos.h
index 3ed1381..0d90be8 100644
--- a/protos.h
+++ b/protos.h
@@ -21,11 +21,31 @@
 #ifndef PROTOS_H
 #define PROTOS_H 1
 
-#include <stdint.h>
-#include <stdbool.h>
-#include <stddef.h>
-#include <string.h>
-
+/* Stand-alone definitions for various types, compatible with
+   the Alpha Linux ABI and GCC.  This eliminates dependencies
+   on external headers.  */
+typedef unsigned char  uint8_t;
+typedef unsigned short uint16_t;
+typedef unsigned int   uint32_t;
+typedef unsigned long  uint64_t;
+typedef unsigned long  size_t;
+
+#define bool           _Bool
+#define true           1
+#define false          0
+
+#define offsetof(type, member) __builtin_offsetof(type, member)
+
+typedef __builtin_va_list va_list;
+#define va_start(ap, last)     __builtin_va_start((ap), (last))
+#define va_arg                 __builtin_va_arg
+#define va_end(ap)             __builtin_va_end(ap)
+
+#define NULL                   ((void *)0)
+
+extern void *memset(void *, int, size_t);
+extern void *memcpy(void *, const void *, size_t);
+extern size_t strlen(const char *);
 
 /*
  * Call_Pal functions.
-- 
2.30.2

Re: [PATCH 1/8] Make qemu-palcode build environment standalone. NFC.

[Richard Henderson]

On 6/2/21 8:53 PM, Jason Thorpe wrote:
> Don't include system headers.  Instead, provide standalone definitions
> and declarations of types needed and functions used by the PALcode that
> are compatible with the standard Alpha / GCC ABI.
> 
> Signed-off-by: Jason Thorpe<thorpej@me.com>
> ---
>   init.c   |  2 --
>   memcpy.c |  2 +-
>   memset.c |  2 +-
>   printf.c |  4 +---
>   protos.h | 30 +++++++++++++++++++++++++-----
>   5 files changed, 28 insertions(+), 12 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 2/8] Fix delivery of unaligned access exceptions.

[Jason Thorpe]

In the unaligned access exception vector, actually pass the return PC
in the exception frame.  This is required in order for unaligned access
fixup handlers in the operating system to work.

Signed-off-by: Jason Thorpe <thorpej@me.com>
---
 pal.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pal.S b/pal.S
index 64941a8..015a829 100644
--- a/pal.S
+++ b/pal.S
@@ -278,7 +278,7 @@ Pal_Unalign:
 	blbs	p6, MchkBugCheck
 	addq	p6, 4, p6		// increment past the faulting insn
 
-	STACK_FRAME p0, p1, p2, 1
+	STACK_FRAME p0, p6, p2, 1
 
 	mfpr	p0, ptEntUna
 	mfpr	$gp, ptKgp
-- 
2.30.2

Re: [PATCH 2/8] Fix delivery of unaligned access exceptions.

[Richard Henderson]

On 6/2/21 8:53 PM, Jason Thorpe wrote:
> In the unaligned access exception vector, actually pass the return PC
> in the exception frame.  This is required in order for unaligned access
> fixup handlers in the operating system to work.
> 
> Signed-off-by: Jason Thorpe<thorpej@me.com>
> ---
>   pal.S | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Ouch.  Good catch.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 3/8] Fix initialization of the hwrpb.hwrpb.cpuid field.

[Jason Thorpe]

Initialize the hwrpb.hwrpb.cpuid field with the primary CPU ID, not
the processor type, as per the architecture specification.  Some
operating systems check and assert this.

Improve a couple of comments.

Signed-off-by: Jason Thorpe <thorpej@me.com>
---
 init.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/init.c b/init.c
index 429a9ad..b3919b6 100644
--- a/init.c
+++ b/init.c
@@ -141,6 +141,7 @@ init_hwrpb (unsigned long memsize, unsigned long cpus)
   unsigned long pal_pages;
   unsigned long amask;
   unsigned long i;
+  unsigned long proc_type = EV4_CPU;
   
   hwrpb.hwrpb.phys_addr = PA(&hwrpb);
 
@@ -162,12 +163,12 @@ init_hwrpb (unsigned long memsize, unsigned long cpus)
   switch (__builtin_alpha_implver())
     {
     case 0: /* EV4 */
-      hwrpb.hwrpb.cpuid = EV4_CPU;
+      proc_type = EV4_CPU;
       hwrpb.hwrpb.max_asn = 63;
       break;
 
     case 1: /* EV5 */
-      hwrpb.hwrpb.cpuid
+      proc_type
 	= ((amask & 0x101) == 0x101 ? PCA56_CPU		/* MAX+BWX */
 	   : amask & 1 ? EV56_CPU			/* BWX */
 	   : EV5_CPU);
@@ -175,11 +176,16 @@ init_hwrpb (unsigned long memsize, unsigned long cpus)
       break;
 
     case 2: /* EV6 */
-      hwrpb.hwrpb.cpuid = (amask & 4 ? EV67_CPU : EV6_CPU);  /* CIX */
+      proc_type = (amask & 4 ? EV67_CPU : EV6_CPU);     /* CIX */
       hwrpb.hwrpb.max_asn = 255;
       break;
     }
 
+  /* This field is the WHAMI of the primary CPU.  Just initialize
+     this to 0; CPU #0 is always the primary on real Alpha systems
+     (except for the TurboLaser).  */
+  hwrpb.hwrpb.cpuid = 0;
+
   hwrpb.hwrpb.pagesize = PAGE_SIZE;
   hwrpb.hwrpb.pa_bits = 40;
   hwrpb.hwrpb.sys_type = SYS_TYPE;
@@ -187,9 +193,18 @@ init_hwrpb (unsigned long memsize, unsigned long cpus)
   hwrpb.hwrpb.sys_revision = SYS_REVISION;
   for (i = 0; i < cpus; ++i)
     {
-      /* ??? Look up these bits.  Snagging the value examined by the kernel. */
+      /* Set the following PCS flags:
+	 (bit 2) Processor Available
+	 (bit 3) Processor Present
+	 (bit 6) PALcode Valid
+	 (bit 7) PALcode Memory Valid
+	 (bit 8) PALcode Loaded
+
+	 ??? We really should be intializing the PALcode memory and
+	 scratch space fields if we're setting PMV, or not set PMV,
+	 but Linux checks for it, so...  */
       hwrpb.processor[i].flags = 0x1cc;
-      hwrpb.processor[i].type = hwrpb.hwrpb.cpuid;
+      hwrpb.processor[i].type = proc_type;
     }
 
   hwrpb.hwrpb.intr_freq = HZ * 4096;
@@ -257,8 +272,8 @@ init_i8259 (void)
   outb(0x04, PORT_PIC1_DATA);	/* ICW3: slave control INTC2 */
   outb(0x01, PORT_PIC1_DATA);	/* ICW4 */
 
-  /* Initialize level triggers.  The CY82C693UB that's on real alpha
-     hardware doesn't have this; this is a PIIX extension.  However,
+  /* Initialize level triggers.  The CY82C693UB that's on some real alpha
+     systems controls these differently; we assume a PIIX here.  However,
      QEMU doesn't implement regular level triggers.  */
   outb(0xff, PORT_PIC2_ELCR);
   outb(0xff, PORT_PIC1_ELCR);
-- 
2.30.2

Re: [PATCH 3/8] Fix initialization of the hwrpb.hwrpb.cpuid field.

[Richard Henderson]

On 6/2/21 8:53 PM, Jason Thorpe wrote:
> Initialize the hwrpb.hwrpb.cpuid field with the primary CPU ID, not
> the processor type, as per the architecture specification.  Some
> operating systems check and assert this.
> 
> Improve a couple of comments.
> 
> Signed-off-by: Jason Thorpe<thorpej@me.com>
> ---
>   init.c | 29 ++++++++++++++++++++++-------
>   1 file changed, 22 insertions(+), 7 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


> @@ -257,8 +272,8 @@ init_i8259 (void)
>    outb(0x04, PORT_PIC1_DATA);	/* ICW3: slave control INTC2 */
>    outb(0x01, PORT_PIC1_DATA);	/* ICW4 */
>  
> -  /* Initialize level triggers.  The CY82C693UB that's on real alpha
> -     hardware doesn't have this; this is a PIIX extension.  However,
> +  /* Initialize level triggers.  The CY82C693UB that's on some real alpha
> +     systems controls these differently; we assume a PIIX here.  However,
>       QEMU doesn't implement regular level triggers.  */
>    outb(0xff, PORT_PIC2_ELCR);
>    outb(0xff, PORT_PIC1_ELCR);

I'll split this out to a separate patch.


r~

[PATCH 4/8] Make some PCI macros available to other files. NFC.

[Jason Thorpe]

Move PCI_DEVFN(), PCI_BUS(), PCI_SLOT(), and PCI_FUNC() to pci.h.

Signed-off-by: Jason Thorpe <thorpej@me.com>
---
 pci.c | 4 ----
 pci.h | 5 +++++
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/pci.c b/pci.c
index 87a101c..ba05adb 100644
--- a/pci.c
+++ b/pci.c
@@ -31,10 +31,6 @@
 #include "pci_regs.h"
 
 
-#define PCI_DEVFN(slot, func)	((((slot) & 0x1f) << 3) | ((func) & 0x07))
-#define PCI_BUS(devfn)		((devfn) >> 8)
-#define PCI_SLOT(devfn)		(((devfn) >> 3) & 0x1f)
-#define PCI_FUNC(devfn)		((devfn) & 0x07)
 #define PCI_SLOT_MAX		32
 #define PCI_FUNC_MAX		8
 #define PCI_REGION_ROM		6
diff --git a/pci.h b/pci.h
index b751c6f..b4a4f80 100644
--- a/pci.h
+++ b/pci.h
@@ -60,6 +60,11 @@ extern void pci_config_maskw(int bdf, int addr, uint16_t off, uint16_t on);
 
 extern int pci_next(int bdf, int *pmax);
 
+#define PCI_DEVFN(slot, func)	((((slot) & 0x1f) << 3) | ((func) & 0x07))
+#define PCI_BUS(devfn)		((devfn) >> 8)
+#define PCI_SLOT(devfn)		(((devfn) >> 3) & 0x1f)
+#define PCI_FUNC(devfn)		((devfn) & 0x07)
+
 #define foreachpci(BDF, MAX)				\
 	for (MAX = 0x0100, BDF = pci_next(0, &MAX);	\
 	     BDF >= 0;					\
-- 
2.30.2

Re: [PATCH 4/8] Make some PCI macros available to other files. NFC.

[Richard Henderson]

On 6/2/21 8:53 PM, Jason Thorpe wrote:
> Move PCI_DEVFN(), PCI_BUS(), PCI_SLOT(), and PCI_FUNC() to pci.h.
> 
> Signed-off-by: Jason Thorpe<thorpej@me.com>
> ---
>   pci.c | 4 ----
>   pci.h | 5 +++++
>   2 files changed, 5 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 5/8] Fix incorrect initialization of PCI BARs.

[Jason Thorpe]

Only program a BAR as a 64-bit MEM BAR if it really is a 64-bit MEM BAR.
Fixes an issue with the CMD646 IDE controller under NetBSD.

Improve some debug/info messages.

Signed-off-by: Jason Thorpe <thorpej@me.com>
---
 pci.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/pci.c b/pci.c
index ba05adb..5e9c906 100644
--- a/pci.c
+++ b/pci.c
@@ -84,7 +84,7 @@ pci_setup_device(int bdf, uint32_t *p_io_base, uint32_t *p_mem_base)
   device_id = pci_config_readw(bdf, PCI_DEVICE_ID);
   class_id = pci_config_readw(bdf, PCI_CLASS_DEVICE);
 
-  printf("PCI: %02x:%02x:%x class %04x id %04x:%04x\r\n",
+  printf("PCI: %d:%d:%d class %04x id %04x:%04x\r\n",
 	 PCI_BUS(bdf), PCI_SLOT(bdf), PCI_FUNC(bdf),
          class_id, vendor_id, device_id);
 
@@ -118,9 +118,10 @@ pci_setup_device(int bdf, uint32_t *p_io_base, uint32_t *p_mem_base)
 	  *p_base = addr + size;
 	  pci_config_writel(bdf, ofs, addr);
 
-	  printf("PCI:   region %d: %08x\r\n", region, addr);
+	  printf("PCI:   region %d (BAR %02x): %08x\r\n", region, ofs, addr);
 
-	  if ((val & PCI_BASE_ADDRESS_MEM_TYPE_MASK)
+	  if ((old & PCI_BASE_ADDRESS_SPACE_IO) == 0 &&
+	      (old & PCI_BASE_ADDRESS_MEM_TYPE_MASK)
 	      == PCI_BASE_ADDRESS_MEM_TYPE_64)
 	    {
 	      pci_config_writel(bdf, ofs + 4, 0);
-- 
2.30.2

Re: [PATCH 5/8] Fix incorrect initialization of PCI BARs.

[Philippe Mathieu-Daudé]

Hi Jason,

On 6/3/21 5:53 AM, Jason Thorpe wrote:
> Only program a BAR as a 64-bit MEM BAR if it really is a 64-bit MEM BAR.
> Fixes an issue with the CMD646 IDE controller under NetBSD.

OK for this fix,

> 
> Improve some debug/info messages.

but I'm not sure why you changed the format.
Better split in 2 patches.

> 
> Signed-off-by: Jason Thorpe <thorpej@me.com>
> ---
>  pci.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/pci.c b/pci.c
> index ba05adb..5e9c906 100644
> --- a/pci.c
> +++ b/pci.c
> @@ -84,7 +84,7 @@ pci_setup_device(int bdf, uint32_t *p_io_base, uint32_t *p_mem_base)
>    device_id = pci_config_readw(bdf, PCI_DEVICE_ID);
>    class_id = pci_config_readw(bdf, PCI_CLASS_DEVICE);
>  
> -  printf("PCI: %02x:%02x:%x class %04x id %04x:%04x\r\n",
> +  printf("PCI: %d:%d:%d class %04x id %04x:%04x\r\n",
>  	 PCI_BUS(bdf), PCI_SLOT(bdf), PCI_FUNC(bdf),
>           class_id, vendor_id, device_id);
>  
> @@ -118,9 +118,10 @@ pci_setup_device(int bdf, uint32_t *p_io_base, uint32_t *p_mem_base)
>  	  *p_base = addr + size;
>  	  pci_config_writel(bdf, ofs, addr);
>  
> -	  printf("PCI:   region %d: %08x\r\n", region, addr);
> +	  printf("PCI:   region %d (BAR %02x): %08x\r\n", region, ofs, addr);
>  
> -	  if ((val & PCI_BASE_ADDRESS_MEM_TYPE_MASK)
> +	  if ((old & PCI_BASE_ADDRESS_SPACE_IO) == 0 &&
> +	      (old & PCI_BASE_ADDRESS_MEM_TYPE_MASK)
>  	      == PCI_BASE_ADDRESS_MEM_TYPE_64)
>  	    {
>  	      pci_config_writel(bdf, ofs + 4, 0);
> 

Re: [PATCH 5/8] Fix incorrect initialization of PCI BARs.

[Jason Thorpe]

> On Jun 3, 2021, at 2:24 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> 
> Hi Jason,
> 
> On 6/3/21 5:53 AM, Jason Thorpe wrote:
>> Only program a BAR as a 64-bit MEM BAR if it really is a 64-bit MEM BAR.
>> Fixes an issue with the CMD646 IDE controller under NetBSD.
> 
> OK for this fix,
> 
>> 
>> Improve some debug/info messages.
> 
> but I'm not sure why you changed the format.
> Better split in 2 patches.

I'm not particularly wedded to the printf format changes.  I tweaked them when I was debugging the problem.  I'll update the patch with the printf format changes elided.

-- thorpej

Re: [PATCH 5/8] Fix incorrect initialization of PCI BARs.

[Richard Henderson]

On 6/2/21 8:53 PM, Jason Thorpe wrote:
> -	  if ((val & PCI_BASE_ADDRESS_MEM_TYPE_MASK)
> +	  if ((old & PCI_BASE_ADDRESS_SPACE_IO) == 0 &&

The correct test is

   (old & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_MEMORY

Bitwise it's the same thing. I'll fix it up while applying.


r~

[PATCH 6/8] Provide interrupt mapping information in PCI config registers.

[Jason Thorpe]

Use system-specific information to program the interrupt line register
with the interrupt mappings, which is what the SRM console does on real
hardware; some operating systems (e.g. NetBSD) use this information
rather than having interrupt mappings tables for every possible system
variation.

Signed-off-by: Jason Thorpe <thorpej@me.com>
---
 pci.c         | 20 +++++++++++++++++++-
 sys-clipper.h | 27 +++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/pci.c b/pci.c
index 5e9c906..e3cab26 100644
--- a/pci.c
+++ b/pci.c
@@ -29,6 +29,7 @@
 #include "protos.h"
 #include "pci.h"
 #include "pci_regs.h"
+#include SYSTEM_H
 
 
 #define PCI_SLOT_MAX		32
@@ -132,7 +133,24 @@ pci_setup_device(int bdf, uint32_t *p_io_base, uint32_t *p_mem_base)
 
   pci_config_maskw(bdf, PCI_COMMAND, 0, PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
 
-  /* Map the interrupt.  */
+  /* Map the interrupt and program the IRQ into the line register.
+     Some operating systems rely on the Console providing this information
+     in order to avoid having mapping tables for every possible system
+     variation.  */
+
+  const uint8_t pin = pci_config_readb(bdf, PCI_INTERRUPT_PIN);
+  const uint8_t slot = PCI_SLOT(bdf);
+  const int irq = MAP_PCI_INTERRUPT(slot, pin, class_id);
+
+  if (irq == -1)
+    {
+      /* No interrupt mapping.  */
+      pci_config_writeb(bdf, PCI_INTERRUPT_LINE, 0xff);
+    }
+  else
+    {
+      pci_config_writeb(bdf, PCI_INTERRUPT_LINE, irq);
+    }
 }
 
 void
diff --git a/sys-clipper.h b/sys-clipper.h
index 31094ff..4f563bf 100644
--- a/sys-clipper.h
+++ b/sys-clipper.h
@@ -27,4 +27,31 @@
 #define SYS_VARIATION	(5 << 10)
 #define SYS_REVISION	0
 
+#ifndef __ASSEMBLER__
+
+#define MAP_PCI_INTERRUPT(SLOT, PIN, CLASS_ID)			\
+({								\
+  int IRQ;							\
+								\
+  if (CLASS_ID == 0x0601)					\
+    {								\
+      /* PCI-ISA bridge is hard-wired to IRQ 55 on real		\
+         hardware, and comes in at a different SCB vector;	\
+         force the line register to -1.  */			\
+      IRQ = -1;							\
+    }								\
+  else if (PIN >= 1 && PIN <= 4)				\
+    {								\
+      /* See hw/alpha/dp264.c:clipper_pci_map_irq()  */		\
+      IRQ = (SLOT + 1) * 4 + (PIN - 1);				\
+    }								\
+  else								\
+    {								\
+      IRQ = -1;							\
+    }								\
+  IRQ;								\
+})
+
+#endif /* ! __ASSEMBLER__ */
+
 #endif
-- 
2.30.2

Re: [PATCH 6/8] Provide interrupt mapping information in PCI config registers.

[Richard Henderson]

On 6/2/21 8:53 PM, Jason Thorpe wrote:
> Use system-specific information to program the interrupt line register
> with the interrupt mappings, which is what the SRM console does on real
> hardware; some operating systems (e.g. NetBSD) use this information
> rather than having interrupt mappings tables for every possible system
> variation.
> 
> Signed-off-by: Jason Thorpe <thorpej@me.com>

Thanks.

> +  /* Map the interrupt and program the IRQ into the line register.
> +     Some operating systems rely on the Console providing this information
> +     in order to avoid having mapping tables for every possible system
> +     variation.  */
> +
> +  const uint8_t pin = pci_config_readb(bdf, PCI_INTERRUPT_PIN);
> +  const uint8_t slot = PCI_SLOT(bdf);
> +  const int irq = MAP_PCI_INTERRUPT(slot, pin, class_id);
> +
> +  if (irq == -1)
> +    {
> +      /* No interrupt mapping.  */
> +      pci_config_writeb(bdf, PCI_INTERRUPT_LINE, 0xff);
> +    }
> +  else
> +    {
> +      pci_config_writeb(bdf, PCI_INTERRUPT_LINE, irq);
> +    }

I've folded this non-distinction into the functional interface.

> +#define MAP_PCI_INTERRUPT(SLOT, PIN, CLASS_ID)			\

I've turned this into a static inline.


r~

[PATCH 7/8] Provide a Console Terminal Block in the HWRPB.

[Jason Thorpe]

Provide a Console Terminal Block in the HWRPB so that operating systems
that depend on it can correctly initialize the console device.  In addition
to defining and filling out the structures, there are a couple of other
key changes:

- Redefine the a2 register passed by Qemu at start-up to also include
  some configuration flags, in addition to the CPU count, and define
  a flag to mirror the "-nographics" option.

- We need to initialize the HWRPB *after* initializing VGA, so that
  we'll know if a VGA device is present and in which slot for filling
  out the CTB.

Signed-off-by: Jason Thorpe <thorpej@me.com>
---
 hwrpb.h  | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 init.c   | 32 +++++++++++++++++++++++++++++---
 protos.h |  2 ++
 vgaio.c  |  2 ++
 4 files changed, 87 insertions(+), 3 deletions(-)

diff --git a/hwrpb.h b/hwrpb.h
index 2166bad..1531e68 100644
--- a/hwrpb.h
+++ b/hwrpb.h
@@ -146,6 +146,60 @@ struct crb_struct {
 	struct vf_map_struct map[1];
 };
 
+struct ctb_struct {
+	unsigned long type;
+	unsigned long unit;
+	unsigned long res0;
+	unsigned long len;
+	unsigned long ipl;
+	unsigned long tintr_vec;
+	unsigned long rintr_vec;
+	unsigned long term_type;
+	unsigned long keybd_type;
+	unsigned long keybd_trans;
+	unsigned long keybd_map;
+	unsigned long keybd_state;
+	unsigned long keybd_last;
+	unsigned long font_us;
+	unsigned long font_mcs;
+	unsigned long font_width;
+	unsigned long font_height;
+	unsigned long mon_width;
+	unsigned long mon_height;
+	unsigned long dpi;
+	unsigned long planes;
+	unsigned long cur_width;
+	unsigned long cur_height;
+	unsigned long head_cnt;
+	unsigned long opwindow;
+	unsigned long head_offset;
+	unsigned long putchar;
+	unsigned long io_state;
+	unsigned long listen_state;
+	unsigned long xaddr;
+	unsigned long turboslot;
+	unsigned long server_off;
+	unsigned long line_off;
+	unsigned char csd;
+};
+
+#define CTB_NONE	0x00
+#define CTB_PRINTERPORT	0x02
+#define CTB_GRAPHICS	0x03
+#define CTB_TYPE4	0x04
+
+/*
+ * Format of the Console Terminal Block Type 4 `turboslot' field:
+ *
+ *  63                   40 39       32 31     24 23      16 15   8 7    0
+ *  |      reserved        |  channel  |  hose   | bus type |  bus | slot |
+ */
+
+#define CTB_TURBOSLOT_TYPE_TC           0       /* TURBOchannel */
+#define CTB_TURBOSLOT_TYPE_ISA          1       /* ISA */
+#define CTB_TURBOSLOT_TYPE_EISA         2       /* EISA */
+#define CTB_TURBOSLOT_TYPE_PCI          3       /* PCI */
+
 struct memclust_struct {
 	unsigned long start_pfn;
 	unsigned long numpages;
diff --git a/init.c b/init.c
index b3919b6..aee5cef 100644
--- a/init.c
+++ b/init.c
@@ -36,11 +36,20 @@
 
 #define HZ	1024
 
+/* Upon entry, register a2 contains configuration information from the VM:
+
+   bits 0-5 -- ncpus
+   bit  6   -- "nographics" option (used to initialize CTB)  */
+
+#define CONFIG_NCPUS(x)      ((x) & 63)
+#define CONFIG_NOGRAPHICS(x) ((x) & (1ull << 6))
+
 struct hwrpb_combine {
   struct hwrpb_struct hwrpb;
   struct percpu_struct processor[4];
   struct memdesc_struct md;
   struct memclust_struct mc[2];
+  struct ctb_struct ctb;
   struct crb_struct crb;
   struct procdesc_struct proc_dispatch;
   struct procdesc_struct proc_fixup;
@@ -59,6 +68,8 @@ struct hwrpb_combine hwrpb __attribute__((aligned(PAGE_SIZE)));
 
 void *last_alloc;
 bool have_vga;
+unsigned int pci_vga_bus;
+unsigned int pci_vga_dev;
 
 static void *
 alloc (unsigned long size, unsigned long align)
@@ -136,12 +147,13 @@ init_page_table(void)
 }
 
 static void
-init_hwrpb (unsigned long memsize, unsigned long cpus)
+init_hwrpb (unsigned long memsize, unsigned long config)
 {
   unsigned long pal_pages;
   unsigned long amask;
   unsigned long i;
   unsigned long proc_type = EV4_CPU;
+  unsigned long cpus = CONFIG_NCPUS(config);
   
   hwrpb.hwrpb.phys_addr = PA(&hwrpb);
 
@@ -226,6 +238,19 @@ init_hwrpb (unsigned long memsize, unsigned long cpus)
   hwrpb.mc[1].start_pfn = pal_pages;
   hwrpb.mc[1].numpages = (memsize >> PAGE_SHIFT) - pal_pages;
 
+  hwrpb.hwrpb.ctbt_offset = offsetof(struct hwrpb_combine, ctb);
+  hwrpb.hwrpb.ctb_size = sizeof(hwrpb.ctb);
+  if (have_vga && !CONFIG_NOGRAPHICS(config))
+    {
+      hwrpb.ctb.term_type = CTB_GRAPHICS;
+      hwrpb.ctb.turboslot = (CTB_TURBOSLOT_TYPE_PCI << 16) |
+			    (pci_vga_bus << 8) | pci_vga_dev;
+    }
+  else
+    {
+      hwrpb.ctb.term_type = CTB_PRINTERPORT;
+    }
+
   hwrpb.hwrpb.crb_offset = offsetof(struct hwrpb_combine, crb);
   hwrpb.crb.dispatch_va = &hwrpb.proc_dispatch;
   hwrpb.crb.dispatch_pa = PA(&hwrpb.proc_dispatch);
@@ -300,18 +325,19 @@ swppal(void *entry, void *pcb)
 }
 
 void
-do_start(unsigned long memsize, void (*kernel_entry)(void), unsigned long cpus)
+do_start(unsigned long memsize, void (*kernel_entry)(void),
+         unsigned long config)
 {
   last_alloc = _end;
 
   init_page_table();
-  init_hwrpb(memsize, cpus);
   init_pcb();
   init_i8259();
   uart_init();
   ps2port_setup();
   pci_setup();
   vgahw_init();
+  init_hwrpb(memsize, config);
 
   swppal(kernel_entry ? kernel_entry : do_console, &pcb);
 }
diff --git a/protos.h b/protos.h
index 0d90be8..44ad233 100644
--- a/protos.h
+++ b/protos.h
@@ -222,6 +222,8 @@ extern unsigned long crb_fixup(unsigned long vptptr, unsigned long hwrpb);
  */
 
 extern bool have_vga;
+extern unsigned int pci_vga_bus;
+extern unsigned int pci_vga_dev;
 
 extern void do_console(void);
 extern void entInt(void);
diff --git a/vgaio.c b/vgaio.c
index 2dd7eb7..1fb0d52 100644
--- a/vgaio.c
+++ b/vgaio.c
@@ -570,6 +570,8 @@ vgahw_init(void)
 
  found:
   have_vga = 1;
+  pci_vga_bus = PCI_BUS(bdf);
+  pci_vga_dev = PCI_SLOT(bdf);
 
   vmode_g = find_vga_entry(3);
 
-- 
2.30.2

Re: [PATCH 7/8] Provide a Console Terminal Block in the HWRPB.

[Richard Henderson]

On 6/2/21 8:53 PM, Jason Thorpe wrote:
> +  hwrpb.hwrpb.ctbt_offset = offsetof(struct hwrpb_combine, ctb);
> +  hwrpb.hwrpb.ctb_size = sizeof(hwrpb.ctb);
> +  if (have_vga && !CONFIG_NOGRAPHICS(config))
> +    {
> +      hwrpb.ctb.term_type = CTB_GRAPHICS;
> +      hwrpb.ctb.turboslot = (CTB_TURBOSLOT_TYPE_PCI << 16) |
> +			    (pci_vga_bus << 8) | pci_vga_dev;
> +    }
> +  else
> +    {
> +      hwrpb.ctb.term_type = CTB_PRINTERPORT;
> +    }

I'm concerned that you're initializing only 1 or 2 slots of 34.

It would seem that at a bare minimum the struct should be zeroed, and the 
device-independent header (4 slots) should be set.

I notice you're setting term_type (offset 56) and not type (offset 0), which is 
where my documentation says that CTB_GRAPHICS goes (Console Interface 
Architecture 2.3.8.2 Console Terminal Block Table).

I'm also confused that this

> + * Format of the Console Terminal Block Type 4 `turboslot' field:

says "type 4", but you're actually using type 3 (GRAPHICS) above.

But I do see that what you're filling in is exactly what netbsd examines -- no 
header checks, no size checks, or anything.  And that openbsd has an exact copy 
of that code.


r~

Re: [PATCH 7/8] Provide a Console Terminal Block in the HWRPB.

[Jason Thorpe]

> On Jun 6, 2021, at 12:27 PM, Richard Henderson <richard.henderson@linaro.org> wrote:
> 
> On 6/2/21 8:53 PM, Jason Thorpe wrote:
>> +  hwrpb.hwrpb.ctbt_offset = offsetof(struct hwrpb_combine, ctb);
>> +  hwrpb.hwrpb.ctb_size = sizeof(hwrpb.ctb);
>> +  if (have_vga && !CONFIG_NOGRAPHICS(config))
>> +    {
>> +      hwrpb.ctb.term_type = CTB_GRAPHICS;
>> +      hwrpb.ctb.turboslot = (CTB_TURBOSLOT_TYPE_PCI << 16) |
>> +			    (pci_vga_bus << 8) | pci_vga_dev;
>> +    }
>> +  else
>> +    {
>> +      hwrpb.ctb.term_type = CTB_PRINTERPORT;
>> +    }
> 
> I'm concerned that you're initializing only 1 or 2 slots of 34.
> 
> It would seem that at a bare minimum the struct should be zeroed, and the device-independent header (4 slots) should be set.

I'll rework it.

> I notice you're setting term_type (offset 56) and not type (offset 0), which is where my documentation says that CTB_GRAPHICS goes (Console Interface Architecture 2.3.8.2 Console Terminal Block Table).

It could be that the value was mirrored in both fields.  I'll investigate further.

> I'm also confused that this
> 
>> + * Format of the Console Terminal Block Type 4 `turboslot' field:
> 
> says "type 4", but you're actually using type 3 (GRAPHICS) above.

Yes.  The GRAPHICS type was originally just for the TURBOchannel systems, but when the first AlphaStations landed, SRM continued using GRAPHICS as the "term_type" ... it's entirely possible that the "type" field was in fact set to MULTIPURPOSE in that case.

> But I do see that what you're filling in is exactly what netbsd examines -- no header checks, no size checks, or anything.  And that openbsd has an exact copy of that code.

I'll see if I can figure out what Digital Unix does.

-- thorpej

[PATCH 8/8] Fixes for seconday CPU start-up.

[Jason Thorpe]

Changes to make secondary CPU start-up work on NetBSD, which depends
on some specific behavior in the architecture specification:

- Change the internal swppal() function to take the new VPTPTR and
  Procedure Value as explicit arguments.  Adapt do_start() to the
  new the new swppal() signature.

- In do_start_wait(), extract the new VPTPTR and PV from the relevant
  HWRPB fields, which will have been initialized by the OS, and pass
  them to swppal().

- In the SWPPAL PAL call, get the value to stuff into PV (r27) from
  a4 (r20), and add a comment describing why this implementation detail
  is allowed by the architecture specification.

Signed-off-by: Jason Thorpe <thorpej@me.com>
---
 init.c | 25 ++++++++++++++++---------
 pal.S  | 13 ++++++++++---
 2 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/init.c b/init.c
index aee5cef..bfe4d96 100644
--- a/init.c
+++ b/init.c
@@ -313,14 +313,16 @@ init_i8259 (void)
 }
 
 static void __attribute__((noreturn))
-swppal(void *entry, void *pcb)
+swppal(void *entry, void *pcb, unsigned long vptptr, unsigned long pv)
 {
   register int variant __asm__("$16") = 2;	/* OSF/1 PALcode */
   register void *pc __asm__("$17") = entry;
   register unsigned long pa_pcb __asm__("$18") = PA(pcb);
-  register unsigned long vptptr __asm__("$19") = VPTPTR;
+  register unsigned long newvptptr __asm__("$19") = vptptr;
+  register unsigned long newpv __asm__("$20") = pv;
 
-  asm("call_pal 0x0a" : : "r"(variant), "r"(pc), "r"(pa_pcb), "r"(vptptr));
+  asm("call_pal 0x0a" : :
+      "r"(variant), "r"(pc), "r"(pa_pcb), "r"(newvptptr), "r"(newpv));
   __builtin_unreachable ();
 }
 
@@ -339,7 +341,9 @@ do_start(unsigned long memsize, void (*kernel_entry)(void),
   vgahw_init();
   init_hwrpb(memsize, config);
 
-  swppal(kernel_entry ? kernel_entry : do_console, &pcb);
+  void *new_pc = kernel_entry ? kernel_entry : do_console;
+
+  swppal(new_pc, &pcb, VPTPTR, (unsigned long)new_pc);
 }
 
 void
@@ -354,14 +358,16 @@ do_start_wait(unsigned long cpuid)
 	{
 	  /* ??? The only message I know of is "START\r\n".
 	     I can't be bothered to verify more than 4 characters.  */
-	  /* ??? The Linux kernel fills in, but does not require,
-	     CPU_restart_data.  It just sets that to the same address
-	     as CPU_restart itself.  Our swppal *does* put the PC into
-	     $26 and $27, the latter of which the kernel does rely upon.  */
+
+	  /* Use use a private extension to SWPPAL to get the
+	     CPU_restart_data into $27.  Linux fills it in, but does
+	     not require it. Other operating systems, however, do use
+	     CPU_restart_data as part of secondary CPU start-up.  */
 
 	  unsigned int len = hwrpb.processor[cpuid].ipc_buffer[0];
 	  unsigned int msg = hwrpb.processor[cpuid].ipc_buffer[1];
 	  void *CPU_restart = hwrpb.hwrpb.CPU_restart;
+	  unsigned long CPU_restart_data = hwrpb.hwrpb.CPU_restart_data;
 	  __sync_synchronize();
 	  hwrpb.hwrpb.rxrdy = 0;
 
@@ -369,7 +375,8 @@ do_start_wait(unsigned long cpuid)
 	    {
 	      /* Set bootstrap in progress */
 	      hwrpb.processor[cpuid].flags |= 1;
-	      swppal(CPU_restart, hwrpb.processor[cpuid].hwpcb);
+	      swppal(CPU_restart, hwrpb.processor[cpuid].hwpcb,
+		     hwrpb.hwrpb.vptb, CPU_restart_data);
 	    }
 	}
     }
diff --git a/pal.S b/pal.S
index 015a829..7e3a62c 100644
--- a/pal.S
+++ b/pal.S
@@ -566,6 +566,8 @@ ENDFN	CallPal_Cserve_Cont
  *	r17 (a1) = New PC
  *	r18 (a2) = New PCB
  *	r19 (a3) = New VptPtr
+ *	r20 (a4) = New Procedure Value (to place into $27)
+ *	           (Non-standard; See note below.)
  * 
  * OUTPUT PARAMETERS:
  *
@@ -574,10 +576,15 @@ ENDFN	CallPal_Cserve_Cont
  *			1 - Unknown PALcode variant
  *			2 - Known PALcode variant, but PALcode not loaded
  *
- *	r26 (ra) = r27 (pv) = New PC
+ *	r26 (ra) = New PC
+ *	r27 (pv) = From r20
  *		Note that this is non-architected, but is relied on by
  *		the usage of SwpPal within our own console code in order
- *		to simplify its use within C code.
+ *		to simplify its use within C code.  We can get away with
+ *		the extra non-standard argument (in $20) because as
+ *		architected, all registers except SP and R0 are
+ *		UNPREDICTABLE; therefore private internal usage is
+ *		fine.
  *
  */
 	ORG_CALL_PAL_PRIV(0x0A)
@@ -624,7 +631,7 @@ CallPal_SwpPal_Cont:
 	mtpr	$31, qemu_tbia		// Flush TLB for new PTBR
 
 	mov	a1, $26
-	mov	a1, $27
+	mov	a4, $27
 	hw_ret	(a1)
 ENDFN	CallPal_SwpPal_Cont
 	.previous
-- 
2.30.2

Re: [PATCH 8/8] Fixes for seconday CPU start-up.

[Richard Henderson]

On 6/2/21 8:53 PM, Jason Thorpe wrote:
> Changes to make secondary CPU start-up work on NetBSD, which depends
> on some specific behavior in the architecture specification:
> 
> - Change the internal swppal() function to take the new VPTPTR and
>    Procedure Value as explicit arguments.  Adapt do_start() to the
>    new the new swppal() signature.
> 
> - In do_start_wait(), extract the new VPTPTR and PV from the relevant
>    HWRPB fields, which will have been initialized by the OS, and pass
>    them to swppal().
> 
> - In the SWPPAL PAL call, get the value to stuff into PV (r27) from
>    a4 (r20), and add a comment describing why this implementation detail
>    is allowed by the architecture specification.
> 
> Signed-off-by: Jason Thorpe<thorpej@me.com>
> ---
>   init.c | 25 ++++++++++++++++---------
>   pal.S  | 13 ++++++++++---
>   2 files changed, 26 insertions(+), 12 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 0/8] PALcode fixes required to run NetBSD/alpha.

[Philippe Mathieu-Daudé]

Cc'ing Richard & Emilio.

On 6/3/21 5:53 AM, Jason Thorpe wrote:
> Included here are a set of patches that fix issues in qemu-palcode needed
> to run NetBSD/alpha under Qemu.  Some fix garden-variety bugs, some fix
> deviations from the architecture specification or behavior of the SRM
> console on real Alpha hardware.
> 
> Two of the changes (patch 6 and patch 7) also require other fixes in
> Qemu itself, which will be submitted separately.  However, the changes
> are fully compatible with existing Qemu alpha VMs because Linux does
> not use the the SRM PCI interrupt mapping information (it has its own
> tables for the system variations it supports) or the Console Terminal
> Block in the HWRPB.
> 
> Jason Thorpe (8):
>   Make qemu-palcode build environment standalone. NFC.
>   Fix delivery of unaligned access exceptions.
>   Fix initialization of the hwrpb.hwrpb.cpuid field.
>   Make some PCI macros available to other files.  NFC.
>   Fix incorrect initialization of PCI BARs.
>   Provide interrupt mapping information in PCI config registers.
>   Provide a Console Terminal Block in the HWRPB.
>   Fixes for seconday CPU start-up.
> 
>  hwrpb.h       | 54 +++++++++++++++++++++++++++++++
>  init.c        | 88 +++++++++++++++++++++++++++++++++++++++------------
>  memcpy.c      |  2 +-
>  memset.c      |  2 +-
>  pal.S         | 15 ++++++---
>  pci.c         | 31 +++++++++++++-----
>  pci.h         |  5 +++
>  printf.c      |  4 +--
>  protos.h      | 30 +++++++++++++++---
>  sys-clipper.h | 27 ++++++++++++++++
>  vgaio.c       |  2 ++
>  11 files changed, 218 insertions(+), 42 deletions(-)
> 

Re: [PATCH 0/8] PALcode fixes required to run NetBSD/alpha.

[Richard Henderson]

On 6/2/21 8:53 PM, Jason Thorpe wrote:
> Jason Thorpe (8):
>    Make qemu-palcode build environment standalone. NFC.
>    Fix delivery of unaligned access exceptions.
>    Fix initialization of the hwrpb.hwrpb.cpuid field.
>    Make some PCI macros available to other files.  NFC.
>    Fix incorrect initialization of PCI BARs.
>    Provide interrupt mapping information in PCI config registers.
>    Provide a Console Terminal Block in the HWRPB.
>    Fixes for seconday CPU start-up.

Thanks.  Applied all but the Console Terminal Block patch, and pushed to my 
upstream github repo.

I'll wait for an update on the ctb patch before refreshing the image in the 
qemu repo.


r~