Page MenuHomeFreeBSD
Differential D24041 Diff 83075 stand/i386/libi386/biosacpi.c

Changeset View

Standalone View

View Options

stand/i386/libi386/biosacpi.c

Show All 32 Lines
#include <btxv86.h> #include <btxv86.h>
#include "libi386.h" #include "libi386.h"
#include "platform/acfreebsd.h" #include "platform/acfreebsd.h"
#include "acconfig.h" #include "acconfig.h"
#define ACPI_SYSTEM_XFACE #define ACPI_SYSTEM_XFACE
#include "actypes.h" #include "actypes.h"
#include "actbl.h" #include "actbl.h"
#include "actbl3.h"
/* /*
* Detect ACPI and export information about the ACPI BIOS into the * Detect ACPI and export information about the ACPI BIOS into the
* environment. * environment.
*/ */
static ACPI_TABLE_RSDP *biosacpi_find_rsdp(void); static ACPI_TABLE_RSDP *biosacpi_find_rsdp(void);
static ACPI_TABLE_RSDP *biosacpi_search_rsdp(char *base, int length); static ACPI_TABLE_RSDP *biosacpi_search_rsdp(char *base, int length);
#define RSDP_CHECKSUM_LENGTH 20 #define RSDP_CHECKSUM_LENGTH 20
static ACPI_TABLE_SPCR *
find_spcr(ACPI_TABLE_RSDP *rsdp)
{
unsigned count, i;
ACPI_TABLE_XSDT *xsdt;
ACPI_TABLE_RSDT *rsdt;
void *ptr;
cemUnsubmitted
Done
Inline Actions

Usually in FreeBSD we declare variables at top of function. (style)

cem: Usually in FreeBSD we declare variables at top of function. (style)
if (rsdp->Revision >= 2 && rsdp->XsdtPhysicalAddress != 0) {
xsdt = (ACPI_TABLE_XSDT *)PTOV(rsdp->XsdtPhysicalAddress);
count = xsdt->Header.Length - sizeof(ACPI_TABLE_HEADER);
count /= sizeof(xsdt->TableOffsetEntry[0]);
cemUnsubmitted
Done
Inline Actions

Perhaps sizeof(xsdt->TableOffsetEntry[0]), as that is where the uint64 comes from?

Similarly below for rdst fallback.

cem: Perhaps `sizeof(xsdt->TableOffsetEntry[0])`, as that is where the uint64 comes from? Similarly…
for (i = 0; i < count; i++) {
ptr = PTOV(xsdt->TableOffsetEntry[i]);
if (memcmp(ptr, ACPI_SIG_SPCR,
sizeof(ACPI_SIG_SPCR) - 1) == 0) {
return (ptr);
cemUnsubmitted
Done
Inline Actions

ACPI_SIG_SPCR is defined as the literal string "SPCR", which has sizeof() 5, not the probably expected 4.

Semantically, it might make be more logical to type the pointer as ACPI_TABLE_SPCR only after doing the signature check. That'd avoid the need to NULL it out on mismatches but is otherwise just a subjective and non-functional change.

E.g.,

void *tmp;
for ( ... ) {
        tmp = (void *)PTOV(xsdt->TableOffsetEntry[i];
        if (memcmp(tmp, ACPI_SIG_SPCR, strlen(ACPI_SIG_SPCR)) != 0)
                continue;
        return (tmp); // implicit cast to ACPI_TABLE_SPCR* here
}
cem: `ACPI_SIG_SPCR` is defined as the literal string `"SPCR"`, which has `sizeof()` //5//, not the…
}
}
}
if (rsdp->RsdtPhysicalAddress != 0) {
rsdt = (ACPI_TABLE_RSDT *)PTOV(rsdp->RsdtPhysicalAddress);
count = rsdt->Header.Length - sizeof(ACPI_TABLE_HEADER);
count /= sizeof(rsdt->TableOffsetEntry[0]);
for (i = 0; i < count; i++) {
ptr = PTOV(rsdt->TableOffsetEntry[i]);
if (memcmp(ptr, ACPI_SIG_SPCR,
sizeof(ACPI_SIG_SPCR) - 1) == 0) {
return (ptr);
}
}
}
return (NULL);
}
/*
* Find and parse SPCR table to set up serial console.
*/
static void
biosacpi_setup_spcr(ACPI_TABLE_SPCR *spcr)
{
unsigned baudrate;
char value[21];
if (spcr == NULL)
return;
switch (spcr->BaudRate) {
cemUnsubmitted
Done
Inline Actions

Address is typically small (0x3f8), but the value is technically uint64 and we need 21 chars to store the maximal possible decimal value (18446744073709551615), plus trailing \0.

cem: Address is typically small (0x3f8), but the value is technically uint64 and we need 21 chars to…
case 0:
baudrate = 0;
break;
case 3:
baudrate = 9600;
break;
case 4:
baudrate = 19200;
break;
case 6:
baudrate = 57600;
break;
case 7:
baudrate = 115200;
break;
default:
return;
}
switch (spcr->SerialPort.SpaceId) {
case ACPI_ADR_SPACE_SYSTEM_IO:
setenv("console", "comconsole", 1);
snprintf(value, sizeof(value), "%ju",
impUnsubmitted
Not Done
Inline Actions

I think it's wrong to unconditionally set this. It's a POLA violation and change in behavior that people will not like.

imp: I think it's wrong to unconditionally set this. It's a POLA violation and change in behavior…
(uintmax_t)spcr->SerialPort.Address);
setenv("comconsole_port", value, 1);
break;
impUnsubmitted
Not Done
Inline Actions

Likewise. I know the port I want if I've set it. Overriding it is uncool.

imp: Likewise. I know the port I want if I've set it. Overriding it is uncool.
default:
/* XXX not implemented. */
return;
cemUnsubmitted
Done
Inline Actions

I think we want "%ju" and (uintmax_t)spcr->SerialPort.Address, as we define ACPI_USE_SYSTEM_INTTYPES in at least some stand targets and use the FreeBSD uint64 type, which is not always long long. (Clang complains about type mismatch even when long and long long are both 64 bits.)

cem: I think we want `"%ju"` and `(uintmax_t)spcr->SerialPort.Address`, as we define…
}
if (baudrate > 0) {
snprintf(value, sizeof(value), "%u", baudrate);
setenv("comconsole_speed", value, 1);
}
impUnsubmitted
Not Done
Inline Actions

Likewise with speed.

imp: Likewise with speed.
}
void void
biosacpi_detect(void) biosacpi_detect(void)
{ {
ACPI_TABLE_RSDP *rsdp; ACPI_TABLE_RSDP *rsdp;
char buf[24]; char buf[24];
int revision; int revision;
/* locate and validate the RSDP */ /* locate and validate the RSDP */
Show All 30 Linesbiosacpi_detect(void)
/* XXX extended checksum? */ /* XXX extended checksum? */
sprintf(buf, "0x%016llx", rsdp->XsdtPhysicalAddress); sprintf(buf, "0x%016llx", rsdp->XsdtPhysicalAddress);
setenv("hint.acpi.0.xsdt", buf, 1); setenv("hint.acpi.0.xsdt", buf, 1);
setenv("acpi.xsdt", buf, 1); setenv("acpi.xsdt", buf, 1);
sprintf(buf, "%d", rsdp->Length); sprintf(buf, "%d", rsdp->Length);
setenv("hint.acpi.0.xsdt_length", buf, 1); setenv("hint.acpi.0.xsdt_length", buf, 1);
setenv("acpi.xsdt_length", buf, 1); setenv("acpi.xsdt_length", buf, 1);
} }
biosacpi_setup_spcr(find_spcr(rsdp));
} }
/* /*
* Find the RSDP in low memory. See section 5.2.2 of the ACPI spec. * Find the RSDP in low memory. See section 5.2.2 of the ACPI spec.
*/ */
static ACPI_TABLE_RSDP * static ACPI_TABLE_RSDP *
biosacpi_find_rsdp(void) biosacpi_find_rsdp(void)
{ {
Show All 39 Lines