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