[SeaBIOS] Re: [SeaBIOS PATCH] xen: require Xen info structure at 0x1000 to detect Xen
On Thu, 2023-02-02 at 10:10 +0100, Gerd Hoffmann wrote: > > Thanks, Kevin. > > > > I'd like to get the rest of the Xen platform support in to qemu 8.0 > > if > > possible. Which is currently scheduled for March. > > > > Is there likely to be a SeaBIOS release before then which Gerd > > would > > pull into qemu anyway, or should I submit a submodule update to a > > snapshot of today's tree? That would just pull in this commit, and > > the > > one other fix that's in the SeaBIOS tree since 1.16.1? > > Tagging 1.16.2 in time for the qemu 8.0 should not be a problem given > that we have only bugfixes in master. Roughly around soft freeze is > probably a good time for that. That's, erm, at the *end* of today 2023-03-07, and the freeze happens only *after* Paul has reviewed the phase 2 Xen PV back end support, right? https://wiki.qemu.org/Planning/8.0 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] Re: [SeaBIOS PATCH] xen: require Xen info structure at 0x1000 to detect Xen
On Wed, 2023-02-01 at 21:13 -0500, Kevin O'Connor wrote: > On Fri, Jan 20, 2023 at 11:33:19AM +0000, David Woodhouse wrote: > > From: David Woodhouse > > > > When running under Xen, hvmloader places a table at 0x1000 with the e820 > > information and BIOS tables. If this isn't present, SeaBIOS will > > currently panic. > > > > We now have support for running Xen guests natively in QEMU/KVM, which > > boots SeaBIOS directly instead of via hvmloader, and does not provide > > the same structure. > > > > As it happens, this doesn't matter on first boot. because although we > > set PlatformRunningOn to PF_QEMU|PF_XEN, reading it back again still > > gives zero. Presumably because in true Xen, this is all already RAM. But > > in QEMU with a faithfully-emulated PAM config in the host bridge, it's > > still in ROM mode at this point so we don't see what we've just written. > > > > On reboot, however, the region *is* set to RAM mode and we do see the > > updated value of PlatformRunningOn, do manage to remember that we've > > detected Xen in CPUID, and hit the panic. > > > > It's not trivial to detect QEMU vs. real Xen at the time xen_preinit() > > runs, because it's so early. We can't even make a XENVER_extraversion > > hypercall to look for hints, because we haven't set up the hypercall > > page (and don't have an allocator to give us a page in which to do so). > > > > So just make Xen detection contingent on the info structure being > > present. If it wasn't, we were going to panic anyway. That leaves us > > taking the standard QEMU init path for Xen guests in native QEMU, > > which is just fine. > > Thanks. I committed this change. > > -Kevin Thanks, Kevin. I'd like to get the rest of the Xen platform support in to qemu 8.0 if possible. Which is currently scheduled for March. Is there likely to be a SeaBIOS release before then which Gerd would pull into qemu anyway, or should I submit a submodule update to a snapshot of today's tree? That would just pull in this commit, and the one other fix that's in the SeaBIOS tree since 1.16.1? $ git shortlog rel-1.16.1.. David Woodhouse (1): xen: require Xen info structure at 0x1000 to detect Xen Qi Zhou (1): usb: fix wrong init of keyboard/mouse's if first interface is not boot protocol smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] Re: [PATCH 1/4] better kvm detection
On Mon, 2022-11-21 at 11:32 +0100, Gerd Hoffmann wrote: > In case kvm emulates features of another hypervisor (for example hyperv) > two VMM CPUID blocks will be present, one for the emulated hypervisor > and one for kvm itself. That isn't the case for emulating Xen on KVM, FWIW. Only for Hyper-V on KVM (and also for Hyper-V on Xen). smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] [SeaBIOS PATCH] xen: require Xen info structure at 0x1000 to detect Xen
From: David Woodhouse
When running under Xen, hvmloader places a table at 0x1000 with the e820
information and BIOS tables. If this isn't present, SeaBIOS will
currently panic.
We now have support for running Xen guests natively in QEMU/KVM, which
boots SeaBIOS directly instead of via hvmloader, and does not provide
the same structure.
As it happens, this doesn't matter on first boot. because although we
set PlatformRunningOn to PF_QEMU|PF_XEN, reading it back again still
gives zero. Presumably because in true Xen, this is all already RAM. But
in QEMU with a faithfully-emulated PAM config in the host bridge, it's
still in ROM mode at this point so we don't see what we've just written.
On reboot, however, the region *is* set to RAM mode and we do see the
updated value of PlatformRunningOn, do manage to remember that we've
detected Xen in CPUID, and hit the panic.
It's not trivial to detect QEMU vs. real Xen at the time xen_preinit()
runs, because it's so early. We can't even make a XENVER_extraversion
hypercall to look for hints, because we haven't set up the hypercall
page (and don't have an allocator to give us a page in which to do so).
So just make Xen detection contingent on the info structure being
present. If it wasn't, we were going to panic anyway. That leaves us
taking the standard QEMU init path for Xen guests in native QEMU,
which is just fine.
Untested on actual Xen but ObviouslyCorrect™.
Signed-off-by: David Woodhouse
---
src/fw/xen.c | 45 -
1 file changed, 32 insertions(+), 13 deletions(-)
diff --git a/src/fw/xen.c b/src/fw/xen.c
index a215b9e..00e4b0c 100644
--- a/src/fw/xen.c
+++ b/src/fw/xen.c
@@ -40,16 +40,25 @@ struct xen_seabios_info {
u32 e820_nr;
} PACKED;
-static void validate_info(struct xen_seabios_info *t)
+static struct xen_seabios_info *validate_info(void)
{
-if ( memcmp(t->signature, "XenHVMSeaBIOS", 14) )
-panic("Bad Xen info signature\n");
+struct xen_seabios_info *t = (void *)INFO_PHYSICAL_ADDRESS;
-if ( t->length < sizeof(struct xen_seabios_info) )
-panic("Bad Xen info length\n");
+if ( memcmp(t->signature, "XenHVMSeaBIOS", 14) ) {
+dprintf(1, "Bad Xen info signature\n");
+return NULL;
+}
+
+if ( t->length < sizeof(struct xen_seabios_info) ) {
+dprintf(1, "Bad Xen info length\n");
+return NULL;
+}
-if (checksum(t, t->length) != 0)
-panic("Bad Xen info checksum\n");
+if (checksum(t, t->length) != 0) {
+dprintf(1, "Bad Xen info checksum\n");
+return NULL;
+}
+return t;
}
void xen_preinit(void)
@@ -86,7 +95,10 @@ void xen_preinit(void)
dprintf(1, "No Xen hypervisor found.\n");
return;
}
-PlatformRunningOn = PF_QEMU|PF_XEN;
+if (validate_info())
+PlatformRunningOn = PF_QEMU|PF_XEN;
+else
+dprintf(1, "Not enabling Xen support due to lack of Xen info\n");
}
static int hypercall_xen_version( int cmd, void *arg)
@@ -122,10 +134,14 @@ void xen_hypercall_setup(void)
void xen_biostable_setup(void)
{
-struct xen_seabios_info *info = (void *)INFO_PHYSICAL_ADDRESS;
-void **tables = (void*)info->tables;
+struct xen_seabios_info *info = validate_info();
+void **tables;
int i;
+if (!info)
+panic("Xen info corrupted\n");
+
+tables = (void*)info->tables;
dprintf(1, "xen: copy BIOS tables...\n");
for (i=0; itables_nr; i++)
copy_table(tables[i]);
@@ -136,12 +152,15 @@ void xen_biostable_setup(void)
void xen_ramsize_preinit(void)
{
int i;
-struct xen_seabios_info *info = (void *)INFO_PHYSICAL_ADDRESS;
-struct e820entry *e820 = (struct e820entry *)info->e820;
-validate_info(info);
+struct xen_seabios_info *info = validate_info();
+struct e820entry *e820;
+
+if (!info)
+panic("Xen info corrupted\n");
dprintf(1, "xen: copy e820...\n");
+e820 = (struct e820entry *)info->e820;
for (i = 0; i < info->e820_nr; i++) {
struct e820entry *e = [i];
e820_add(e->start, e->size, e->type);
--
2.34.1
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] Re: [PATCH v2] nvme: Clean up nvme_cmd_readwrite()
On Thu, 2020-11-05 at 16:09 +, David Woodhouse wrote:
> From: David Woodhouse
>
> This ended up with an odd mix of recursion (albeit *mostly*
> tail-recursion) and interation that could have been prettier. In
> addition, while recursing it potentially adjusted op->count which is
> used by callers to see the amount of I/O actually performed.
>
> Fix it by bringing nvme_build_prpl() into the normal loop using 'i'
> as the offset in the op.
>
> Fixes: 94f0510dc ("nvme: Split requests by maximum allowed size")
> Signed-off-by: David Woodhouse
> ---
> v2: Fix my bug with checking a u16 for being < 0.
> Fix the bug I inherited from commit 94f0510dc but made unconditional.
>
> src/hw/nvme.c | 77 ++-
> 1 file changed, 33 insertions(+), 44 deletions(-)
>
Now, on top of that we *could* do something like this...
--- a/src/hw/nvme.c
+++ b/src/hw/nvme.c
@@ -742,6 +742,15 @@ nvme_cmd_readwrite(struct nvme_namespace *ns, struct
disk_op_s *op, int write)
blocks = blocks_remaining < max_blocks ? blocks_remaining
: max_blocks;
+if (blocks < blocks_remaining && ns->block_size < NVME_PAGE_SIZE &&
+!(((u32)op_buf) & (ns->block_size-1))) {
+u32 align = ((u32)op_buf & (NVME_PAGE_SIZE - 1)) /
ns->block_size;
+if (blocks > (max_blocks - align)) {
+dprintf(3, "Restrain to %u blocks to align (buf %p
size %u/%u)\n", max_blocks - align, op_buf, NVME_PAGE_SIZE, ns->block_size);
+blocks = max_blocks - align;
+}
+}
+
if (write) {
memcpy(ns->dma_buffer, op_buf, blocks * ns->block_size);
}
While debugging this I watched a boot sector, after being loaded at
:7c00, load another 63 sectors at :7e00. It didn't get to use
the prpl at all, and looked something like this...
ns 1 read lba 0+1: 0
Booting from :7c00
ns 1 read lba 1+8: 0
ns 1 read lba 9+8: 0
ns 1 read lba 17+8: 0
ns 1 read lba 25+8: 0
ns 1 read lba 33+8: 0
ns 1 read lba 41+8: 0
ns 1 read lba 49+8: 0
ns 1 read lba 57+7: 0
If we make an *attempt* to align it, such as the proof-of-concept shown
above, then it ends up getting back in sync:
ns 1 read lba 0+1: 0
Booting from :7c00
Restrain to 1 blocks to align (buf 0x7e00 size 4096/512)
ns 1 read lba 1+1: 0
ns 1 read lba 1+62: 0
I just don't know that I care enough about the optimisation to make it
worth the ugliness of that special case in the loop, which includes a
division.
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] [PATCH v2] nvme: Clean up nvme_cmd_readwrite()
From: David Woodhouse
This ended up with an odd mix of recursion (albeit *mostly*
tail-recursion) and interation that could have been prettier. In
addition, while recursing it potentially adjusted op->count which is
used by callers to see the amount of I/O actually performed.
Fix it by bringing nvme_build_prpl() into the normal loop using 'i'
as the offset in the op.
Fixes: 94f0510dc ("nvme: Split requests by maximum allowed size")
Signed-off-by: David Woodhouse
---
v2: Fix my bug with checking a u16 for being < 0.
Fix the bug I inherited from commit 94f0510dc but made unconditional.
src/hw/nvme.c | 77 ++-
1 file changed, 33 insertions(+), 44 deletions(-)
diff --git a/src/hw/nvme.c b/src/hw/nvme.c
index cc37bca..f26b811 100644
--- a/src/hw/nvme.c
+++ b/src/hw/nvme.c
@@ -470,30 +470,31 @@ static int nvme_add_prpl(struct nvme_namespace *ns, u64
base)
return 0;
}
-int nvme_build_prpl(struct nvme_namespace *ns, struct disk_op_s *op)
+static int nvme_build_prpl(struct nvme_namespace *ns, void *op_buf, u16 count)
{
int first_page = 1;
-u32 base = (long)op->buf_fl;
-s32 size = op->count * ns->block_size;
+u32 base = (long)op_buf;
+s32 size;
-if (op->count > ns->max_req_size)
-return -1;
+if (count > ns->max_req_size)
+count = ns->max_req_size;
nvme_reset_prpl(ns);
+size = count * ns->block_size;
/* Special case for transfers that fit into PRP1, but are unaligned */
if (((size + (base & ~NVME_PAGE_MASK)) <= NVME_PAGE_SIZE)) {
-ns->prp1 = op->buf_fl;
-return 0;
+ns->prp1 = op_buf;
+return count;
}
/* Every request has to be page aligned */
if (base & ~NVME_PAGE_MASK)
-return -1;
+return 0;
/* Make sure a full block fits into the last chunk */
if (size & (ns->block_size - 1ULL))
-return -1;
+return 0;
for (; size > 0; base += NVME_PAGE_SIZE, size -= NVME_PAGE_SIZE) {
if (first_page) {
@@ -503,10 +504,10 @@ int nvme_build_prpl(struct nvme_namespace *ns, struct
disk_op_s *op)
continue;
}
if (nvme_add_prpl(ns, base))
-return -1;
+return 0;
}
-return 0;
+return count;
}
static int
@@ -725,46 +726,34 @@ nvme_cmd_readwrite(struct nvme_namespace *ns, struct
disk_op_s *op, int write)
{
int res = DISK_RET_SUCCESS;
u16 const max_blocks = NVME_PAGE_SIZE / ns->block_size;
-u16 i;
-
-/* Split up requests that are larger than the device can handle */
-if (op->count > ns->max_req_size) {
-u16 count = op->count;
-
-/* Handle the first max_req_size elements */
-op->count = ns->max_req_size;
-if (nvme_cmd_readwrite(ns, op, write))
-return res;
-
-/* Handle the remainder of the request */
-op->count = count - ns->max_req_size;
-op->lba += ns->max_req_size;
-op->buf_fl += (ns->max_req_size * ns->block_size);
-return nvme_cmd_readwrite(ns, op, write);
-}
-
-if (!nvme_build_prpl(ns, op)) {
-/* Request goes via PRP List logic */
-return nvme_io_readwrite(ns, op->lba, ns->prp1, op->count, write);
-}
+u16 i, blocks;
for (i = 0; i < op->count && res == DISK_RET_SUCCESS;) {
u16 blocks_remaining = op->count - i;
-u16 blocks = blocks_remaining < max_blocks ? blocks_remaining
- : max_blocks;
char *op_buf = op->buf_fl + i * ns->block_size;
-if (write) {
-memcpy(ns->dma_buffer, op_buf, blocks * ns->block_size);
-}
+blocks = nvme_build_prpl(ns, op_buf, blocks_remaining);
+if (blocks) {
+res = nvme_io_readwrite(ns, op->lba + i, ns->prp1, blocks, write);
+dprintf(5, "ns %u %s lba %llu+%u: %d\n", ns->ns_id, write ? "write"
+ : "read",
+op->lba, blocks, res);
+} else {
+blocks = blocks_remaining < max_blocks ? blocks_remaining
+ : max_blocks;
+
+if (write) {
+memcpy(ns->dma_buffer, op_buf, blocks * ns->block_size);
+}
-res = nvme_io_readwrite(ns, op->lba + i, ns->dma_buffer, blocks,
write);
-dprintf(5, "ns %u %s lba %llu+%u: %d\n", ns->ns_id, write ? "write"
- : "read",
-op->lba + i, blocks, res);
+res = nvme_io_readwrite(ns, op->lba + i, ns->dma_buffer, blocks,
write);
[SeaBIOS] Re: [PATCH] nvme: Clean up nvme_cmd_readwrite()
On Wed, 2020-11-04 at 23:27 +, Graf (AWS), Alexander wrote:
> > Am 31.10.2020 um 00:49 schrieb David Woodhouse :
> >
> > From: David woodhouse
> >
> > This ended up with an odd mix of recursion (albeit *mostly* tail-recursion)
> > and interation that could have been prettier. Make it prettier by making
> > nvme_build_prpl() return the number of blocks it consumes, and just
> > looping.
> >
> > If nvme_build_prpl() doesn't want to play, just fall through to the
> > original loop.
> >
> > Signed-off-by: David Woodhouse
>
> Please don't merge just yet. I think we found a bug with this patch,
> but need to find out where exactly.
Found both of them :)
> > @@ -725,34 +726,28 @@ nvme_cmd_readwrite(struct nvme_namespace *ns, struct
> > disk_op_s *op, int write)
> > {
> > int res = DISK_RET_SUCCESS;
> > u16 const max_blocks = NVME_PAGE_SIZE / ns->block_size;
> > -u16 i;
> > +u16 i, blocks;
> >
> > -/* Split up requests that are larger than the device can handle */
> > -if (op->count > ns->max_req_size) {
> > -u16 count = op->count;
> > +while (op->count && ((blocks = nvme_build_prpl(ns, op)) > 0)) {
1. If nvme_build_prpl() returns -1 that becomes (u16)0x and that's >0.
> > +res = nvme_io_readwrite(ns, op->lba, ns->prp1, blocks, write);
> > +dprintf(5, "ns %u %s lba %llu+%u: %d\n", ns->ns_id, write ? "write"
> > + : "read",
> > +op->lba, blocks, res);
> >
> > -/* Handle the first max_req_size elements */
> > -op->count = ns->max_req_size;
> > -if (nvme_cmd_readwrite(ns, op, write))
> > +if (res != DISK_RET_SUCCESS)
> > return res;
> >
> > -/* Handle the remainder of the request */
> > -op->count = count - ns->max_req_size;
> > -op->lba += ns->max_req_size;
> > -op->buf_fl += (ns->max_req_size * ns->block_size);
2. We can't do this. Callers expect op->count to contain the number of
blocks of I/O actually performed...
> > -return nvme_cmd_readwrite(ns, op, write);
> > -}
> > -
> > -if (!nvme_build_prpl(ns, op)) {
> > -/* Request goes via PRP List logic */
> > -return nvme_io_readwrite(ns, op->lba, ns->prp1, op->count, write);
> > +op->count -= blocks;
> > +op->lba += blocks;
> > +op->buf_fl += (blocks * ns->block_size);
... which means that when my cleanup made it happen unconditionally
instead of only in the case where the max size had been exceeded
and it actually had to recurse, it started showing up in the test
runs. But commit 94f0510dc has the problem already.
Running another full test set now, will post the fixed patch when it
completes.
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] [PATCH] nvme: Clean up nvme_cmd_readwrite()
From: David woodhouse
This ended up with an odd mix of recursion (albeit *mostly* tail-recursion)
and interation that could have been prettier. Make it prettier by making
nvme_build_prpl() return the number of blocks it consumes, and just
looping.
If nvme_build_prpl() doesn't want to play, just fall through to the
original loop.
Signed-off-by: David Woodhouse
---
On Fri, 2020-10-30 at 19:09 -0400, Kevin O'Connor wrote:
> FWIW, I agree that a version without recursion (even tail recursion)
> would be nice. If someone wants to test and confirm that, then that
> would be great.
Seems to work here... I just tidied it up a little more and added a
dprintf() to match the original loop. Didn't see it actually limiting
the requests in my testing so I made nvme_build_prpl() limit to 32
blocks and then it did... and still seems to put things in the right
place.
src/hw/nvme.c | 47 +--
1 file changed, 21 insertions(+), 26 deletions(-)
diff --git a/src/hw/nvme.c b/src/hw/nvme.c
index cc37bca..5363d3f 100644
--- a/src/hw/nvme.c
+++ b/src/hw/nvme.c
@@ -472,19 +472,20 @@ static int nvme_add_prpl(struct nvme_namespace *ns, u64
base)
int nvme_build_prpl(struct nvme_namespace *ns, struct disk_op_s *op)
{
-int first_page = 1;
+int first_page = 1, count = op->count;
u32 base = (long)op->buf_fl;
-s32 size = op->count * ns->block_size;
+s32 size;
-if (op->count > ns->max_req_size)
-return -1;
+if (count > ns->max_req_size)
+count = ns->max_req_size;
nvme_reset_prpl(ns);
+size = count * ns->block_size;
/* Special case for transfers that fit into PRP1, but are unaligned */
if (((size + (base & ~NVME_PAGE_MASK)) <= NVME_PAGE_SIZE)) {
ns->prp1 = op->buf_fl;
-return 0;
+return count;
}
/* Every request has to be page aligned */
@@ -506,7 +507,7 @@ int nvme_build_prpl(struct nvme_namespace *ns, struct
disk_op_s *op)
return -1;
}
-return 0;
+return count;
}
static int
@@ -725,34 +726,28 @@ nvme_cmd_readwrite(struct nvme_namespace *ns, struct
disk_op_s *op, int write)
{
int res = DISK_RET_SUCCESS;
u16 const max_blocks = NVME_PAGE_SIZE / ns->block_size;
-u16 i;
+u16 i, blocks;
-/* Split up requests that are larger than the device can handle */
-if (op->count > ns->max_req_size) {
-u16 count = op->count;
+while (op->count && ((blocks = nvme_build_prpl(ns, op)) > 0)) {
+res = nvme_io_readwrite(ns, op->lba, ns->prp1, blocks, write);
+dprintf(5, "ns %u %s lba %llu+%u: %d\n", ns->ns_id, write ? "write"
+ : "read",
+op->lba, blocks, res);
-/* Handle the first max_req_size elements */
-op->count = ns->max_req_size;
-if (nvme_cmd_readwrite(ns, op, write))
+if (res != DISK_RET_SUCCESS)
return res;
-/* Handle the remainder of the request */
-op->count = count - ns->max_req_size;
-op->lba += ns->max_req_size;
-op->buf_fl += (ns->max_req_size * ns->block_size);
-return nvme_cmd_readwrite(ns, op, write);
-}
-
-if (!nvme_build_prpl(ns, op)) {
-/* Request goes via PRP List logic */
-return nvme_io_readwrite(ns, op->lba, ns->prp1, op->count, write);
+op->count -= blocks;
+op->lba += blocks;
+op->buf_fl += (blocks * ns->block_size);
}
for (i = 0; i < op->count && res == DISK_RET_SUCCESS;) {
-u16 blocks_remaining = op->count - i;
-u16 blocks = blocks_remaining < max_blocks ? blocks_remaining
- : max_blocks;
char *op_buf = op->buf_fl + i * ns->block_size;
+u16 blocks_remaining = op->count - i;
+
+blocks = blocks_remaining < max_blocks ? blocks_remaining
+ : max_blocks;
if (write) {
memcpy(ns->dma_buffer, op_buf, blocks * ns->block_size);
--
2.17.1
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] Re: [PATCH v3 4/4] nvme: Split requests by maximum allowed size
On Wed, 2020-10-07 at 12:13 -0400, Kevin O'Connor wrote:
> > > > --- a/src/hw/nvme.c
> > > > +++ b/src/hw/nvme.c
> > > > @@ -727,6 +727,22 @@ nvme_cmd_readwrite(struct nvme_namespace *ns,
> > > > struct disk_op_s *op, int write)
> > > > u16 const max_blocks = NVME_PAGE_SIZE / ns->block_size;
> > > > u16 i;
> > > >
> > > > +/* Split up requests that are larger than the device can handle */
> > > > +if (op->count > ns->max_req_size) {
> > > > +u16 count = op->count;
> > > > +
> > > > +/* Handle the first max_req_size elements */
> > > > +op->count = ns->max_req_size;
> > > > +if (nvme_cmd_readwrite(ns, op, write))
> > > > +return res;
> > > > +
> > > > +/* Handle the remainder of the request */
> > > > +op->count = count - ns->max_req_size;
> > > > +op->lba += ns->max_req_size;
> > > > +op->buf_fl += (ns->max_req_size * ns->block_size);
> > > > +return nvme_cmd_readwrite(ns, op, write);
> > > > +}
> > >
> > > Depending on the disk access, this code could run with a small stack.
> > > I would avoid recursion.
> >
> > This is tail recursion, which any reasonable compiler converts into
> > a jmp :). Take a look at the .o file - for me it did become a plain
> > jump.
> >
>
> Okay, that's fine then.
It's still kind of icky. This used to be a purely iterative function.
Now for a large unaligned request (which nvme_build_prpl refuses to
handle) you get a mixture of (mostly tail) recursion and iteration.
It'll recurse for each max_req_size, then iterate over each page within
that.
I'd much rather see just a simple iterative loop. Something like this
(incremental, completely untested):
--- a/src/hw/nvme.c
+++ b/src/hw/nvme.c
@@ -472,19 +472,20 @@ static int nvme_add_prpl(struct nvme_namespace *ns, u64
base)
int nvme_build_prpl(struct nvme_namespace *ns, struct disk_op_s *op)
{
-int first_page = 1;
+int first_page = 1, count = op->count;
u32 base = (long)op->buf_fl;
-s32 size = op->count * ns->block_size;
+s32 size;
-if (op->count > ns->max_req_size)
-return -1;
+if (count > ns->max_req_size)
+ count = ns->max_req_size;
nvme_reset_prpl(ns);
+size = count * ns->block_size;
/* Special case for transfers that fit into PRP1, but are unaligned */
if (((size + (base & ~NVME_PAGE_MASK)) <= NVME_PAGE_SIZE)) {
ns->prp1 = op->buf_fl;
-return 0;
+return count;
}
/* Every request has to be page aligned */
@@ -506,7 +507,7 @@ int nvme_build_prpl(struct nvme_namespace *ns, struct
disk_op_s *op)
return -1;
}
-return 0;
+return count;
}
static int
@@ -725,27 +726,16 @@ nvme_cmd_readwrite(struct nvme_namespace *ns, struct
disk_op_s *op, int write)
{
int res = DISK_RET_SUCCESS;
u16 const max_blocks = NVME_PAGE_SIZE / ns->block_size;
-u16 i;
-
-/* Split up requests that are larger than the device can handle */
-if (op->count > ns->max_req_size) {
-u16 count = op->count;
+u16 i, count;
-/* Handle the first max_req_size elements */
-op->count = ns->max_req_size;
-if (nvme_cmd_readwrite(ns, op, write))
-return res;
-
-/* Handle the remainder of the request */
-op->count = count - ns->max_req_size;
-op->lba += ns->max_req_size;
-op->buf_fl += (ns->max_req_size * ns->block_size);
-return nvme_cmd_readwrite(ns, op, write);
-}
+while (op->count && ((count = nvme_build_prpl(ns, op)) > 0)) {
+ res = nvme_io_readwrite(ns, op->lba, ns->prp1, count, write);
+if (res != DISK_RET_SUCCESS)
+break;
-if (!nvme_build_prpl(ns, op)) {
-/* Request goes via PRP List logic */
-return nvme_io_readwrite(ns, op->lba, ns->prp1, op->count, write);
+op->count -= count;
+op->lba += count;
+op->buf_fl += (count * ns->block_size);
}
for (i = 0; i < op->count && res == DISK_RET_SUCCESS;) {
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] [PATCH v3] csm: Fix boot priority translation
Explicitly handle the BBS_DO_NOT_BOOT_FROM and BBS_IGNORE_ENTRY values.
Also add one to the other priority values, as find_prio() does for
entries from bootorder. SeaBIOS uses zero for an item explicitly
selected in interactive_bootmenu().
Signed-off-by: David Woodhouse
---
v2: No code change, just correct the commit message.
v3: Add comment in code as requested by agraf, update commit message more.
src/fw/csm.c | 23 ---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/src/fw/csm.c b/src/fw/csm.c
index 3fcc252..8359bcb 100644
--- a/src/fw/csm.c
+++ b/src/fw/csm.c
@@ -319,6 +319,23 @@ handle_csm(struct bregs *regs)
csm_return(regs);
}
+static int csm_prio_to_seabios(u16 csm_prio)
+{
+switch (csm_prio) {
+case BBS_DO_NOT_BOOT_FROM:
+case BBS_IGNORE_ENTRY:
+return -1;
+
+case BBS_LOWEST_PRIORITY:
+case BBS_UNPRIORITIZED_ENTRY:
+default:
+// SeaBIOS default priorities start at 1, with 0 being used for
+// an item explicitly selected from interactive_bootmenu().
+// As in find_prio(), add 1 to the value being returned.
+return csm_prio + 1;
+}
+}
+
int csm_bootprio_ata(struct pci_device *pci, int chanid, int slave)
{
if (!csm_boot_table)
@@ -327,7 +344,7 @@ int csm_bootprio_ata(struct pci_device *pci, int chanid,
int slave)
int index = 1 + (chanid * 2) + slave;
dprintf(3, "CSM bootprio for ATA%d,%d (index %d) is %d\n", chanid, slave,
index, bbs[index].BootPriority);
-return bbs[index].BootPriority;
+return csm_prio_to_seabios(bbs[index].BootPriority);
}
int csm_bootprio_fdc(struct pci_device *pci, int port, int fdid)
@@ -336,7 +353,7 @@ int csm_bootprio_fdc(struct pci_device *pci, int port, int
fdid)
return -1;
BBS_TABLE *bbs = (void *)csm_boot_table->BbsTable;
dprintf(3, "CSM bootprio for FDC is %d\n", bbs[0].BootPriority);
-return bbs[0].BootPriority;
+return csm_prio_to_seabios(bbs[0].BootPriority);
}
int csm_bootprio_pci(struct pci_device *pci)
@@ -350,7 +367,7 @@ int csm_bootprio_pci(struct pci_device *pci)
if (pci->bdf == pci_to_bdf(bbs[i].Bus, bbs[i].Device,
bbs[i].Function)) {
dprintf(3, "CSM bootprio for PCI(%d,%d,%d) is %d\n", bbs[i].Bus,
bbs[i].Device, bbs[i].Function, bbs[i].BootPriority);
-return bbs[i].BootPriority;
+return csm_prio_to_seabios(bbs[i].BootPriority);
}
}
return -1;
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] Re: [PATCH v2] csm: Fix boot priority translation
On Fri, 2019-06-21 at 10:40 +0200, Gerd Hoffmann wrote: > Hi, > > > Our downstream patch for not initialising NVMe controllers if we aren't > > going to boot from them, makes its decision based on the priority. > > What is the state of that patch btw? It's on my "technical debt that I want to kill so we're using a pristine upstream again" list. I'll take a look after I've fixed the final quirks and got CSM boots working seamlessly again. > I remember it being posted a while back, suggestion was to make it > generic (skip init for everything which has a priority higher than > "HALT", not only nvm), but the discussion went silent quickly and IIRC > there never was a v2 of that patch ... If we do that, then those drives won't be available to INT13h at all. We'd be saying that just because we don't want to *boot* from them, DOS can't see them at all. Would we assign INT13h drive numbers to them and initialise them on demand? I don't think that can work because we don't even know if an ATA drive is present, don't know how many NVMe namespaces are present, so couldn't assign numbers accordingly. Perhaps the first INT13h access to a drive that doesn't exist (0x81 normally?) would trigger *all* pending drive probes to happen? Can we even do them that late? Or do we make it an option — at build time or through fw_cfg or something else, to just *not* initialise those drives at all because we know we're not booting DOS today? smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] Re: [PATCH v2] csm: Fix boot priority translation
On Thu, 2019-06-20 at 09:43 -0400, Kevin O'Connor wrote: > On Thu, Jun 20, 2019 at 01:07:45PM +0100, David Woodhouse wrote: > > For CSM, the highest priority for a boot entry is zero. SeaBIOS doesn't > > use zero, and the highest priority is 1. > > FYI, SeaBIOS does treat zero as the highest priority. And a negative > priority means "use default priority". > > I'm fine with the change, though. I don't think find_prio ever returns zero. dprintf(1, "Searching bootorder for: %s\n", glob); int i; for (i = 0; i < BootorderCount; i++) if (glob_prefix(glob, Bootorder[i])) return i+1; return -1; Our downstream patch for not initialising NVMe controllers if we aren't going to boot from them, makes its decision based on the priority. Originally it had 'if (bootprio_find_pci_device(pci) == 1)' to start only the top priority boot device; nowadays it has '> 0' to match all bootable drives. Now, I can fix that patch fairly easily to be '>= 0' now that it no longer wants to match precisely only the first. But in general, it seems that '1' is the top priority so making the CSM values match that seems sensible. smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] [PATCH v2] csm: Fix boot priority translation
For CSM, the highest priority for a boot entry is zero. SeaBIOS doesn't
use zero, and the highest priority is 1.
Make the results of csm_bootprio_*() conform to that convention. Also
explicitly handle the BBS_DO_NOT_BOOT_FROM and BBS_IGNORE_ENTRY values.
Signed-off-by: David Woodhouse
---
v2: No code change, just correct the commit message.
src/fw/csm.c | 20 +---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/src/fw/csm.c b/src/fw/csm.c
index 3fcc252..7663d31 100644
--- a/src/fw/csm.c
+++ b/src/fw/csm.c
@@ -319,6 +319,20 @@ handle_csm(struct bregs *regs)
csm_return(regs);
}
+static int csm_prio_to_seabios(u16 csm_prio)
+{
+switch (csm_prio) {
+case BBS_DO_NOT_BOOT_FROM:
+case BBS_IGNORE_ENTRY:
+return -1;
+
+case BBS_LOWEST_PRIORITY:
+case BBS_UNPRIORITIZED_ENTRY:
+default:
+return csm_prio + 1;
+}
+}
+
int csm_bootprio_ata(struct pci_device *pci, int chanid, int slave)
{
if (!csm_boot_table)
@@ -327,7 +341,7 @@ int csm_bootprio_ata(struct pci_device *pci, int chanid,
int slave)
int index = 1 + (chanid * 2) + slave;
dprintf(3, "CSM bootprio for ATA%d,%d (index %d) is %d\n", chanid, slave,
index, bbs[index].BootPriority);
-return bbs[index].BootPriority;
+return csm_prio_to_seabios(bbs[index].BootPriority);
}
int csm_bootprio_fdc(struct pci_device *pci, int port, int fdid)
@@ -336,7 +350,7 @@ int csm_bootprio_fdc(struct pci_device *pci, int port, int
fdid)
return -1;
BBS_TABLE *bbs = (void *)csm_boot_table->BbsTable;
dprintf(3, "CSM bootprio for FDC is %d\n", bbs[0].BootPriority);
-return bbs[0].BootPriority;
+return csm_prio_to_seabios(bbs[0].BootPriority);
}
int csm_bootprio_pci(struct pci_device *pci)
@@ -350,7 +364,7 @@ int csm_bootprio_pci(struct pci_device *pci)
if (pci->bdf == pci_to_bdf(bbs[i].Bus, bbs[i].Device,
bbs[i].Function)) {
dprintf(3, "CSM bootprio for PCI(%d,%d,%d) is %d\n", bbs[i].Bus,
bbs[i].Device, bbs[i].Function, bbs[i].BootPriority);
-return bbs[i].BootPriority;
+return csm_prio_to_seabios(bbs[i].BootPriority);
}
}
return -1;
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] Re: [PATCH] csm: Fix boot priority translation
On Wed, 2019-06-19 at 12:27 +0100, David Woodhouse wrote: > For CSM, the highest priority is zero. In SeaBIOS that means "don't", and > the highest priority is 1. > > So we end up with the fun outcome that booting from NVMe worked only > when it *wasn't* selected as the primary boot target, because we don't > actually run the nvme_controller_setup() thread for an NVMe controller > if its boot prio is zero. > > Signed-off-by: David Woodhouse Hm, turns out the NVMe hack is something that's only in our tree so for upstream that second paragraph is a lie and can be dropped. It's still a correct change to reflect the fact that SeaBIOS doesn't use zero for the highest priority, and correctly handle BBS_DO_NOT_BOOT_FROM and BBS_IGNORE_ENTRY values. smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] [PATCH] csm: Fix boot priority translation
For CSM, the highest priority is zero. In SeaBIOS that means "don't", and
the highest priority is 1.
So we end up with the fun outcome that booting from NVMe worked only
when it *wasn't* selected as the primary boot target, because we don't
actually run the nvme_controller_setup() thread for an NVMe controller
if its boot prio is zero.
Signed-off-by: David Woodhouse
---
src/fw/csm.c | 20 +---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/src/fw/csm.c b/src/fw/csm.c
index 3fcc252..7663d31 100644
--- a/src/fw/csm.c
+++ b/src/fw/csm.c
@@ -319,6 +319,20 @@ handle_csm(struct bregs *regs)
csm_return(regs);
}
+static int csm_prio_to_seabios(u16 csm_prio)
+{
+switch (csm_prio) {
+case BBS_DO_NOT_BOOT_FROM:
+case BBS_IGNORE_ENTRY:
+return -1;
+
+case BBS_LOWEST_PRIORITY:
+case BBS_UNPRIORITIZED_ENTRY:
+default:
+return csm_prio + 1;
+}
+}
+
int csm_bootprio_ata(struct pci_device *pci, int chanid, int slave)
{
if (!csm_boot_table)
@@ -327,7 +341,7 @@ int csm_bootprio_ata(struct pci_device *pci, int chanid,
int slave)
int index = 1 + (chanid * 2) + slave;
dprintf(3, "CSM bootprio for ATA%d,%d (index %d) is %d\n", chanid, slave,
index, bbs[index].BootPriority);
-return bbs[index].BootPriority;
+return csm_prio_to_seabios(bbs[index].BootPriority);
}
int csm_bootprio_fdc(struct pci_device *pci, int port, int fdid)
@@ -336,7 +350,7 @@ int csm_bootprio_fdc(struct pci_device *pci, int port, int
fdid)
return -1;
BBS_TABLE *bbs = (void *)csm_boot_table->BbsTable;
dprintf(3, "CSM bootprio for FDC is %d\n", bbs[0].BootPriority);
-return bbs[0].BootPriority;
+return csm_prio_to_seabios(bbs[0].BootPriority);
}
int csm_bootprio_pci(struct pci_device *pci)
@@ -350,7 +364,7 @@ int csm_bootprio_pci(struct pci_device *pci)
if (pci->bdf == pci_to_bdf(bbs[i].Bus, bbs[i].Device,
bbs[i].Function)) {
dprintf(3, "CSM bootprio for PCI(%d,%d,%d) is %d\n", bbs[i].Bus,
bbs[i].Device, bbs[i].Function, bbs[i].BootPriority);
-return bbs[i].BootPriority;
+return csm_prio_to_seabios(bbs[i].BootPriority);
}
}
return -1;
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] Re: [PATCH] scripts/buildversion.py: allow version to be overridden
On Tue, 2019-06-18 at 18:48 -0400, Kevin O'Connor wrote: > > The main project has an out-of-tree build, and currently *copies* > > everything from the SeaBIOS submodule (except .git) into the build > > directory and building it there. It creates a .version file with the > > overall version number of the main project. > > > > By setting OUT= and KCONFIG_CONFIG= I can do a proper out of tree build > > from the original read-only SeaBIOS submodule... except for the version > > number. This solves that. > > Okay, thanks for the info. However, since my past experience with > completely custom versioning wasn't positive, I'd prefer the main repo > to encourage using the EXTRAVERSION method instead. Ack. I'll come up with a different approach. Thanks. smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] Re: [PATCH] scripts/buildversion.py: allow version to be overridden
On Thu, 2019-06-13 at 13:33 -0400, Kevin O'Connor wrote: > Can you give some background on how this is intended to be used? > > We used to allow the version string to be overridden, but we found the > results were a bit chaotic - different people chose different names > and it was hard to correlate a bug report to the source of the code. > So, we changed to the scheme detailed at: > > https://www.seabios.org/Build_overview#Distribution_builds I encountered a build system with SeaBIOS in a git submodule of the main project. The main project has an out-of-tree build, and currently *copies* everything from the SeaBIOS submodule (except .git) into the build directory and building it there. It creates a .version file with the overall version number of the main project. By setting OUT= and KCONFIG_CONFIG= I can do a proper out of tree build from the original read-only SeaBIOS submodule... except for the version number. This solves that. smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] [PATCH] scripts/buildversion.py: allow version to be overridden
Signed-off-by: David Woodhouse
---
scripts/buildversion.py | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/scripts/buildversion.py b/scripts/buildversion.py
index 8875497..fc2decd 100755
--- a/scripts/buildversion.py
+++ b/scripts/buildversion.py
@@ -113,7 +113,9 @@ def main():
cleanbuild, toolstr = tool_versions(options.tools)
-ver = git_version()
+ver = os.getenv('SEABIOS_VERSION')
+if not ver:
+ver = git_version()
cleanbuild = cleanbuild and 'dirty' not in ver
if not ver:
ver = file_version()
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] [PATCH v2] csm: Sanitise alignment constraint in Legacy16GetTableAddress
The alignment constraint is defined in the CSM specifications as
"Bit mapped. First non-zero bit from the right is the alignment."
Use __fls() to sanitise the alignment given that definition, since
passing a non-power-of-two alignment to _malloc() isn't going to work
well. And cope with being passed zero, which was happening for the
E820 table allocation from EDK2.
Signed-off-by: David Woodhouse
---
v2: Enforce MALLOC_MIN_ALIGN
src/fw/csm.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/fw/csm.c b/src/fw/csm.c
index 03b4bb8..3fcc252 100644
--- a/src/fw/csm.c
+++ b/src/fw/csm.c
@@ -258,11 +258,21 @@ handle_csm_0006(struct bregs *regs)
u16 region = regs->bx; // (1 for F000 seg, 2 for E000 seg, 0 for either)
void *chunk = NULL;
+dprintf(3, "Legacy16GetTableAddress size %x align %x region %d\n",
+size, align, region);
+
if (!region)
region = 3;
-dprintf(3, "Legacy16GetTableAddress size %x align %x region %d\n",
-size, align, region);
+// DX = Required address alignment. Bit mapped.
+// First non-zero bit from the right is the alignment.*/
+if (align) {
+align = 1 << __ffs(align);
+if (align < MALLOC_MIN_ALIGN)
+align = MALLOC_MIN_ALIGN;
+} else {
+align = MALLOC_MIN_ALIGN;
+}
if (region & 2)
chunk = _malloc(, size, align);
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] [PATCH] csm: Sanitise alignment constraint in Legacy16GetTableAddress
The alignment constraint is defined in the CSM specifications as "Bit mapped. First non-zero bit from the right is the alignment." Use __fls() to sanitise the alignment given that definition, since passing a non-power-of-two alignment to _malloc() isn't going to work well. And cope with being passed zero, which was happening for the E820 table allocation from EDK2. Signed-off-by: David Woodhouse --- src/fw/csm.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/src/fw/csm.c b/src/fw/csm.c index 03b4bb8..bf7b8f5 100644 --- a/src/fw/csm.c +++ b/src/fw/csm.c @@ -264,6 +264,13 @@ handle_csm_0006(struct bregs *regs) dprintf(3, "Legacy16GetTableAddress size %x align %x region %d\n", size, align, region); +// DX = Required address alignment. Bit mapped. +// First non-zero bit from the right is the alignment.*/ +if (align) + align = 1 << __ffs(align); +else + align = 1; + if (region & 2) chunk = _malloc(, size, align); if (!chunk && (region & 1)) smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] Re: Real mode kexec failure with non-IDE disk
On Tue, 2019-04-30 at 22:56 -0400, Kevin O'Connor wrote: > Hi David, > > That call trace certainly looks odd. The SeaBIOS debugging info would > help - try compiling SeaBIOS with debug level 8 and grab the log (as > described at: https://www.seabios.org/Debugging#Diagnostic_information > ). Choosing Xen because it actually succeeds, while Linux real-mode boot then dies a bit later even if I use IDE disks... This SeaBIOS git master (f4c6e4c1) with an IDE disk, from the moment of kexec: unimplemented handle_15XX:330: a=ec00 b=0002 c= d= ds=8f00 es=8f00 ss=d980 si= di= bp= sp=fe8e cs=8f00 ip=0060 f=0246 enter handle_12: a=534d3c00 b=befe c=3c00 d=002ffb80 ds=8f00 es=8f00 ss=d980 si= di=0af1 bp= sp=fe8e cs=8f00 ip=0143 f=0202 invalid handle_legacy_disk:729: a=534d4139 b=55aa c=fe03 d=002ffb81 ds=8f00 es=8f00 ss=d980 si=1539 di= bp= sp=fe8e cs=8f00 ip=015b f=0293 invalid handle_legacy_disk:729: a=534d0201 b= c=889f0001 d=002f0081 ds=8f00 es=8ee0 ss=d980 si=aa55 di= bp= sp=fe8e cs=8f00 ip=0202 f=0247 Xen 4.11.2-pre (XEN) Xen version 4.11.2-pre (d...@ant.amazon.com) (gcc (Ubuntu 7.3.0-27ubuntu1~18.04) 7.3.0) debug=n Wed May 1 22:08:39 IDT 2019 (XEN) Latest ChangeSet: Wed May 1 13:37:09 2019 +0300 git:7c281700a8 (XEN) Bootloader: kexec 2.0.19.git (XEN) Command line: console=vga,com1 (XEN) Xen image load base address: 0 (XEN) Video information: (XEN) VGA is text mode 80x25, font 8x16 (XEN) Disc information: (XEN) Found 1 MBR signatures (XEN) Found 1 EDD information structures And this is all I see when the disk is virtio: unimplemented handle_15XX:330: a=ec00 b=0002 c= d= ds=8f00 es=8f00 ss=d980 si= di= bp= sp=fe8e cs=8f00 ip=0060 f=0246 enter handle_12: a=534d3c00 b=befd c=3c00 d=002ffb40 ds=8f00 es=8f00 ss=d980 si= di=0af1 bp= sp=fe8e cs=8f00 ip=0143 f=0202 invalid handle_legacy_disk:729: a=534d4139 b=55aa c=fe03 d=002ffb81 ds=8f00 es=8f00 ss=d980 si=1539 di= bp= sp=fe8e cs=8f00 ip=015b f=0293 Increasing debug to 9, I see: enter handle_15: a=ec00 b=0002 c= d= ds=8f00 es=8f00 ss=d980 si= di= bp= sp=f96e cs=8f00 ip=0060 f=0246 unimplemented handle_15XX:330: a=ec00 b=0002 c= d= ds=8f00 es=8f00 ss=d980 si= di= bp= sp=f96e cs=8f00 ip=0060 f=0246 enter handle_15: a=e820 b= c=0014 d=534d4150 ds=8f00 es=8f00 ss=d980 si= di=0a51 bp= sp=f96e cs=8f00 ip=00ec f=0246 enter handle_15: a=e820 b=0001 c=0014 d=534d4150 ds=8f00 es=8f00 ss=d980 si= di=0a65 bp= sp=f96e cs=8f00 ip=00ec f=0202 enter handle_15: a=e820 b=0002 c=0014 d=534d4150 ds=8f00 es=8f00 ss=d980 si= di=0a79 bp= sp=f96e cs=8f00 ip=00ec f=0202 enter handle_15: a=e820 b=0003 c=0014 d=534d4150 ds=8f00 es=8f00 ss=d980 si= di=0a8d bp= sp=f96e cs=8f00 ip=00ec f=0206 enter handle_15: a=e820 b=0004 c=0014 d=534d4150 ds=8f00 es=8f00 ss=d980 si= di=0aa1 bp= sp=f96e cs=8f00 ip=00ec f=0202 enter handle_15: a=e820 b=0005 c=0014 d=534d4150 ds=8f00 es=8f00 ss=d980 si= di=0ab5 bp= sp=f96e cs=8f00 ip=00ec f=0206 enter handle_15: a=e820 b=0006 c=0014 d=534d4150 ds=8f00 es=8f00 ss=d980 si= di=0ac9 bp= sp=f96e cs=8f00 ip=00ec f=0206 enter handle_15: a=e820 b=0007 c=0014 d=534d4150 ds=8f00 es=8f00 ss=d980 si= di=0add bp= sp=f96e cs=8f00 ip=00ec f=0202 enter handle_15: a=534d88f1 b= c=0014 d=534d4150 ds=8f00 es=8f00 ss=d980 si= di=0af1 bp= sp=f96e cs=8f00 ip=0112 f=0246 enter handle_15: a=534de801 b= c= d=534d ds=8f00 es=8f00 ss=d980 si= di=0af1 bp= sp=f96e cs=8f00 ip=011f f=0246 enter handle_12: a=534d3c00 b=befd c=3c00 d=002ffb40 ds=8f00 es=8f00 ss=d980 si= di=0af1 bp= sp=f96e cs=8f00 ip=0143 f=0202 invalid handle_legacy_disk:729: a=534d4139 b=55aa c=fe03 d=002ffb81 ds=8f00 es=8f00 ss=d980 si=1539 di= bp= sp=f96e cs=8f00 ip=015b f=0293 call32_smm 0x000ed91a e9120 handle_smi cmd=b5 smbase=0x000a vp notify fe003000 (2) -- 0x0 call16_smm 0x7a79 0 0 handle_smi cmd=b5 smbase=0x000a handle_smi cmd=b5 smbase=0x000a call16_smm 0x7a79 0 0 handle_smi cmd=b5 smbase=0x000a handle_smi cmd=b5 smbase=0x000a call16_smm 0x7a79 0 0 handle_smi cmd=b5 smbase=0x000a handle_smi
[SeaBIOS] Real mode kexec failure with non-IDE disk
When I kexec either Xen or Linux in real mode, from either Xen or Linux, it fails. The last thing I see looks like SeaBIOS trying to use SMM for call32: IN: 0x000f70ec: mov%eax,%esi 0x000f70ef: mov$0xb5,%eax 0x000f70f5: mov$0x1234,%ecx 0x000f70fb: mov$0xef3dc,%ebx 0x000f7101: out%al,$0xb2 0x000f7103: pause IN: 0x000ef3db: hlt This happens when the real mode boot code calls INT 13h to read from the disk. It seems to happen with virtio and SATA disks. This is with the Ubuntu-packaged 1.10.2-1ubuntu1 SeaBIOS. Switching to an IDE disk, or booting with 'edd=skipmbr', makes Xen work and Linux get a little further before it dies anyway. smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] [PATCH 0/4] Reduce use of .code16gcc
This is a gratuitous GCC-ism. For C code actually compiled with GCC we should be using -m16 where it's available (GCC 4.9+). And where the only thing marked with .code16gcc is explicit assembler code, we should just use .code16 and avoid letting the compiler make any of the assumptions that the difference affects. Which, in fact, we already do. (Once upon a time with ancient versions of gas, we needed to use .code16gcc because some instructions just wouldn't compile otherwise. That hasn't been true for a while though.) It still doesn't actually build with clang after this, but it's a bit closer. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] [PATCH 1/4] build: use -m16 where available instead of asm(.code16gcc)
GCC 4.9 and clang 3.5 support the -m16 option on the command line which supersedes the hackish .code16gcc assembler directive. Use it where possible. Signed-off-by: David Woodhouse david.woodho...@intel.com --- Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 78b598e..4c42124 100644 --- a/Makefile +++ b/Makefile @@ -62,13 +62,15 @@ COMMONCFLAGS := -I$(OUT) -Isrc -Os -MD -g \ COMMONCFLAGS += $(call cc-option,$(CC),-nopie,) COMMONCFLAGS += $(call cc-option,$(CC),-fno-stack-protector,) COMMONCFLAGS += $(call cc-option,$(CC),-fno-stack-protector-all,) +COMMA := , CFLAGS32FLAT := $(COMMONCFLAGS) -DMODE16=0 -DMODESEGMENT=0 -fomit-frame-pointer CFLAGSSEG := $(COMMONCFLAGS) -DMODESEGMENT=1 -fno-defer-pop \ $(call cc-option,$(CC),-fno-jump-tables,-DMANUAL_NO_JUMP_TABLE) \ $(call cc-option,$(CC),-fno-tree-switch-conversion,) CFLAGS32SEG := $(CFLAGSSEG) -DMODE16=0 -fomit-frame-pointer -CFLAGS16INC := $(CFLAGSSEG) -DMODE16=1 -Wa,src/code16gcc.s \ +CFLAGS16INC := $(CFLAGSSEG) -DMODE16=1 \ +$(call cc-option,$(CC),-m16,-Wa$(COMMA)src/code16gcc.s) \ $(call cc-option,$(CC),--param large-stack-frame=4,-fno-inline) CFLAGS16 := $(CFLAGS16INC) -fomit-frame-pointer -- 1.9.3 -- David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] [PATCH 3/4] romlayout: Use .code16 not .code16gcc
There's no need to use .code16gcc where we are writing assembler code explicitly. It only affects word-size-ambiguous instructions, and we should just be explicit. And we are. Signed-off-by: David Woodhouse david.woodho...@intel.com --- src/romlayout.S | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/romlayout.S b/src/romlayout.S index 57e8bcc..9c2719e 100644 --- a/src/romlayout.S +++ b/src/romlayout.S @@ -21,7 +21,7 @@ // %edx = return location (in 32bit mode) // Clobbers: ecx, flags, segment registers, cr0, idt/gdt DECLFUNC transition32 -.code16gcc +.code16 transition32: movl %eax, %ecx @@ -102,7 +102,7 @@ transition16big: ljmpw $SEG32_MODE16BIG_CS, $1f -.code16gcc +.code16 1: // Disable protected mode movl %cr0, %eax @@ -145,7 +145,7 @@ __call16big: jmp transition16big // Make call. -.code16gcc +.code16 1: movl $_zonelow_seg, %edx// Adjust %ds, %ss, and %esp movl %edx, %ds movzwl StackSeg, %edx @@ -177,7 +177,7 @@ __call16big: // Far call a 16bit function from 16bit mode with a specified cpu register state // %eax = address of struct bregs, %edx = segment of struct bregs // Clobbers: %e[bc]x, %e[ds]i, flags -.code16gcc +.code16 DECLFUNC __farcall16 __farcall16: // Save %edx/%eax, %ebp @@ -372,7 +372,7 @@ entry_pcibios32: popfl lretl -.code16gcc +.code16 DECLFUNC entry_pcibios16 entry_pcibios16: ENTRY_ARG handle_pcibios @@ -421,7 +421,7 @@ entry_elf: movl $BUILD_STACK_ADDR, %esp ljmpl $SEG32_MODE32_CS, $_cfunc32flat_handle_post -.code16gcc +.code16 // UEFI Compatibility Support Module (CSM) entry point EXPORTFUNC entry_csm @@ -453,7 +453,7 @@ entry_csm: __csm_return: movl $1f, %edx jmp transition16big -.code16gcc +.code16 // Switch back to original stack 1: movzwl BREGS_code+2(%eax), %edx -- 1.9.3 -- David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] [PATCH 4/4] vgaentry: Use .code16 not .code16gcc
There's no need to use .code16gcc where we are writing assembler code explicitly. It only affects word-size-ambiguous instructions, and we should just be explicit. And we are. Signed-off-by: David Woodhouse david.woodho...@intel.com --- vgasrc/vgaentry.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vgasrc/vgaentry.S b/vgasrc/vgaentry.S index 11197f1..3c2c885 100644 --- a/vgasrc/vgaentry.S +++ b/vgasrc/vgaentry.S @@ -15,7 +15,7 @@ / .section .rom.header -.code16gcc +.code16 .global _rom_header, _rom_header_size, _rom_header_checksum _rom_header: .word 0xaa55 -- 1.9.3 -- David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] [PATCH 2/4] smm: Use .code16 not .code16gcc
There's no need to use .code16gcc where we are writing assembler code explicitly. It only affects word-size-ambiguous instructions, and we should just be explicit. And we are. Signed-off-by: David Woodhouse david.woodho...@intel.com --- src/fw/smm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/fw/smm.c b/src/fw/smm.c index 0f59f20..77e1e63 100644 --- a/src/fw/smm.c +++ b/src/fw/smm.c @@ -19,7 +19,7 @@ extern u8 smm_relocation_start, smm_relocation_end; ASM32FLAT( .global smm_relocation_start, smm_relocation_end\n - .code16gcc\n + .code16\n /* code to relocate SMBASE to 0xa */ smm_relocation_start:\n @@ -46,7 +46,7 @@ ASM32FLAT( extern u8 smm_code_start, smm_code_end; ASM32FLAT( .global smm_code_start, smm_code_end\n - .code16gcc\n + .code16\n smm_code_start:\n rsm\n smm_code_end:\n -- 1.9.3 -- David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] [PATCH v4] Update EFI_COMPATIBILITY16_TABLE to match 0.98 spec update
Unless CONFIG_MALLOC_UPPERMEMORY is turned off, we expect to use the
space between the top of option ROMs and the bottom of our own BIOS code
as a stack. OVMF was previously marking the whole region from 0xC to
0xF read-only before invoking our Legacy16Boot method. Read-only
stack considered harmful.
Version 0.98 of the CSM spec adds the UmaAddress and UmaSize fields which
allow the CSM to specify a memory region that needs to be writeable, so
provide that information.
Signed-off-by: David Woodhouse david.woodho...@intel.com
---
src/fw/csm.c | 6 ++
src/std/LegacyBios.h | 20
2 files changed, 26 insertions(+)
diff --git a/src/fw/csm.c b/src/fw/csm.c
index a44ed26..7cdb398 100644
--- a/src/fw/csm.c
+++ b/src/fw/csm.c
@@ -46,8 +46,14 @@ extern void __csm_return(struct bregs *regs) __noreturn;
static void
csm_return(struct bregs *regs)
{
+u32 rommax = rom_get_max();
+extern u8 final_readonly_start[];
+
dprintf(3, handle_csm returning AX=%04x\n, regs-ax);
+csm_compat_table.UmaAddress = rommax;
+csm_compat_table.UmaSize = (u32)final_readonly_start - rommax;
+
PICMask = pic_irqmask_read();
__csm_return(regs);
}
diff --git a/src/std/LegacyBios.h b/src/std/LegacyBios.h
index cf0c3c5..5170c37 100644
--- a/src/std/LegacyBios.h
+++ b/src/std/LegacyBios.h
@@ -228,6 +228,26 @@ typedef struct {
/// Maximum PCI bus number assigned.
///
UINT8 LastPciBus;
+
+ ///
+ /// Start address of UMB RAM
+ ///
+ UINT32UmaAddress;
+
+ ///
+ /// Size of UMB RAM
+ ///
+ UINT32UmaSize;
+
+ ///
+ /// Start address of persistent allocation in high (1MiB) memory
+ ///
+ UINT32HiPermanentMemoryAddress;
+
+ ///
+ /// Size of persistent allocation in high (1MiB) memory
+ ///
+ UINT32HiPermanentMemorySize;
} EFI_COMPATIBILITY16_TABLE;
///
--
1.9.0
--
dwmw2
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH v3] Update EFI_COMPATIBILITY16_TABLE to match 0.98 spec update
On Wed, 2014-05-28 at 09:58 -0400, Kevin O'Connor wrote: Is there an advantage to setting this at compile time vs only setting these fields during runtime? No. Would this work instead? u32 rommax = rom_get_max(); extern u8 final_readonly_start[]; csm_compat_table.UmaAddress = rommax; csm_compat_table.UmaSize = (u32)final_readonly_start - rommax; Yes. New patch to follow... -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH v3] Update EFI_COMPATIBILITY16_TABLE to match 0.98 spec update
On Wed, 2014-05-28 at 09:58 -0400, Kevin O'Connor wrote:
On Tue, May 20, 2014 at 02:22:16PM +0100, David Woodhouse wrote:
Unless CONFIG_MALLOC_UPPERMEMORY is turned off, we expect to use the
space between the top of option ROMs and the bottom of our own BIOS code
as a stack. OVMF was previously marking the whole region from 0xC to
0xF read-only before invoking our Legacy16Boot method. Read-only
stack considered harmful.
Version 0.98 of the CSM spec adds the UmaAddress and UmaSize fields, which
allow the CSM to specify a memory region that needs to be writable, so
provide that information.
[...]
--- a/src/fw/csm.c
+++ b/src/fw/csm.c
@@ -34,6 +34,10 @@ EFI_COMPATIBILITY16_TABLE csm_compat_table VARFSEG
__aligned(16) = {
.Compatibility16CallOffset = 0 /* Filled in by checkrom.py */,
.OemIdStringPointer = (u32)SeaBIOS,
.AcpiRsdPtrPointer = (u32)csm_rsdp,
+#if CONFIG_MALLOC_UPPERMEMORY
+.UmaAddress = (u32)zonelow_base,
+.UmaSize = 0x1,
+#endif
Is there an advantage to setting this at compile time vs only setting
these fields during runtime?
No, I don't believe so. We are invoked with *everything* writeable the
first time, so we don't have to have this filled in.
@@ -49,6 +53,11 @@ csm_return(struct bregs *regs)
dprintf(3, handle_csm returning AX=%04x\n, regs-ax);
PICMask = pic_irqmask_read();
+if (CONFIG_MALLOC_UPPERMEMORY) {
+u32 top = rom_get_max();
+csm_compat_table.UmaAddress = top;
+csm_compat_table.UmaSize = (u32)zonelow_base + 0x1 - top;
+}
Would this work instead?
u32 rommax = rom_get_max();
extern u8 final_readonly_start[];
csm_compat_table.UmaAddress = rommax;
csm_compat_table.UmaSize = (u32)final_readonly_start - rommax;
This should result in the same values as your patch when
CONFIG_MALLOC_UPPERMEMORY. For !CONFIG_MALLOC_UPPERMEMORY it will
result in UmaAddress==final_readonly_start and UmaSize==0.
Looks sane; I'll test it. Perhaps when I get home next week.
--
dwmw2
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [RFC PATCH] Add support for Intel Quark UART.
On Wed, 2014-05-28 at 15:36 -0400, Kevin O'Connor wrote:
On Mon, May 19, 2014 at 07:49:47PM +0100, David Woodhouse wrote:
On Mon, 2014-05-19 at 13:42 -0400, Kevin O'Connor wrote:
I don't see any reference to int 15h, ax=0xd042 as a standard. So,
maybe the author of the above text also wrote their own EFI module
which used that magic value?
Well, the INT 15h call would still be on the BIOS side; that doesn't
sound like an EFI thing at all. Perhaps that's a special-case hack to
make their platform *trigger* the SMM entry, which then gets handled by
the EFI code?
FYI, I just stumbled upon this definition in the std/LegacyBios.h efi
header file:
///
/// SMM_FUNCTION Function constants.
///@{
#define INT15_D0420x
[...]
Looks like it's for BIOS updates. See §8.4 of http://docs.com/Z1P8
--
dwmw2
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [RFC PATCH] Add support for Intel Quark UART.
On Wed, 2014-05-28 at 15:36 -0400, Kevin O'Connor wrote:
On Mon, May 19, 2014 at 07:49:47PM +0100, David Woodhouse wrote:
On Mon, 2014-05-19 at 13:42 -0400, Kevin O'Connor wrote:
I don't see any reference to int 15h, ax=0xd042 as a standard. So,
maybe the author of the above text also wrote their own EFI module
which used that magic value?
Well, the INT 15h call would still be on the BIOS side; that doesn't
sound like an EFI thing at all. Perhaps that's a special-case hack to
make their platform *trigger* the SMM entry, which then gets handled by
the EFI code?
FYI, I just stumbled upon this definition in the std/LegacyBios.h efi
header file:
///
/// SMM_FUNCTION Function constants.
///@{
#define INT15_D0420x
[...]
-Kevin
I've been reading through the EDK2 SMM stuff, and invoking SMM functions
from seems to involve putting parameters into a buffer at a certainly
location, then triggering an SMI. On the EFI side there's obviously a
function you can invoke to *do* it, but we could do it directly.
To make use of it from SeaBIOS-CSM, I think we'd just need to pass in
the location of that buffer. There's space in the EFI_COMPATIBILITY16
table for IBV-specific information.
--
dwmw2
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH v3] Update EFI_COMPATIBILITY16_TABLE to match 0.98 spec update
On Tue, 2014-05-20 at 14:22 +0100, David Woodhouse wrote: However, this doesn't work if I have both CONFIG_MALLOC_UPPERMEMORY *and* CONFIG_EXTRA_STACK enabled. Hm, this appears to be because rom_get_max() is returning 0xef000, causing us to ask UEFI to leave only the range 0xef000-0xf writeable. And that doesn't work quite so nicely when we use the extra stack which in my case is at 0xef520. Is rom_get_max() not what I should be using for this? -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH v3] Update EFI_COMPATIBILITY16_TABLE to match 0.98 spec update
On Wed, 2014-05-21 at 09:52 -0400, Kevin O'Connor wrote: Why is it wrong to declare memory at 0xef000-0xf and have a stack at 0xef520-0xefd20? Er, it's not. I'm stupid. But still it didn't work and it was almost certainly because it's trying to write to read-only memory, given the symptoms and the fact that it doesn't fail when KVM is enabled. I'll run it in qemu with insane levels of tracing, and see if I can work out precisely where it goes wrong. -- David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH v3] Update EFI_COMPATIBILITY16_TABLE to match 0.98 spec update
On Wed, 2014-05-21 at 09:52 -0400, Kevin O'Connor wrote: Why is it wrong to declare memory at 0xef000-0xf and have a stack at 0xef520-0xefd20? I found the problem and it was on the UEFI side, not with the SeaBIOS patch. UEFI was only unlocking the requested 0xef000-0xf region when it actually issued a Legacy16Boot request. Prior to that, the entire 0xc-0x10 region was remaining locked. I'll fix my EDK2 patch accordingly. -- David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] [PATCH v3] Update EFI_COMPATIBILITY16_TABLE to match 0.98 spec update
From d478ac82045a27f82f44ea9ce65f642197fe6078 Mon Sep 17 00:00:00 2001
From: David Woodhouse david.woodho...@intel.com
Date: Fri, 25 Jan 2013 19:39:07 -0600
Subject: [PATCH] Update EFI_COMPATIBILITY16_TABLE to match 0.98 spec update
Unless CONFIG_MALLOC_UPPERMEMORY is turned off, we expect to use the
space between the top of option ROMs and the bottom of our own BIOS code
as a stack. OVMF was previously marking the whole region from 0xC to
0xF read-only before invoking our Legacy16Boot method. Read-only
stack considered harmful.
Version 0.98 of the CSM spec adds the UmaAddress and UmaSize fields, which
allow the CSM to specify a memory region that needs to be writable, so
provide that information.
Signed-off-by: David Woodhouse david.woodho...@intel.com
---
With your incremental patch from 2013-12-02 at 12:46 -0500 merged.
However, this doesn't work if I have both CONFIG_MALLOC_UPPERMEMORY
*and* CONFIG_EXTRA_STACK enabled. Symptoms are the same as the original
problem which both CONFIG_MALLOC_UPPERMEMORY and the CSM spec update set
out to solve: it works only with KVM enabled, because we're writing to
regions which are supposed to be marked as ROM.
Last I see is:
handle_csm returning AX=
UmaAddress ef000, size 1000
InstallProtocolInterface: DB9A1E3D-45CB-4ABB-853B-E5387FDB2E2D 6950C30
tqemu: fatal: Trying to execute code outside RAM or ROM at 0x000a
src/fw/csm.c | 9 +
src/std/LegacyBios.h | 20
2 files changed, 29 insertions(+)
diff --git a/src/fw/csm.c b/src/fw/csm.c
index a44ed26..1f85087 100644
--- a/src/fw/csm.c
+++ b/src/fw/csm.c
@@ -34,6 +34,10 @@ EFI_COMPATIBILITY16_TABLE csm_compat_table VARFSEG
__aligned(16) = {
.Compatibility16CallOffset = 0 /* Filled in by checkrom.py */,
.OemIdStringPointer = (u32)SeaBIOS,
.AcpiRsdPtrPointer = (u32)csm_rsdp,
+#if CONFIG_MALLOC_UPPERMEMORY
+.UmaAddress = (u32)zonelow_base,
+.UmaSize = 0x1,
+#endif
};
EFI_TO_COMPATIBILITY16_INIT_TABLE *csm_init_table;
@@ -49,6 +53,11 @@ csm_return(struct bregs *regs)
dprintf(3, handle_csm returning AX=%04x\n, regs-ax);
PICMask = pic_irqmask_read();
+if (CONFIG_MALLOC_UPPERMEMORY) {
+u32 top = rom_get_max();
+csm_compat_table.UmaAddress = top;
+csm_compat_table.UmaSize = (u32)zonelow_base + 0x1 - top;
+}
__csm_return(regs);
}
diff --git a/src/std/LegacyBios.h b/src/std/LegacyBios.h
index cf0c3c5..5170c37 100644
--- a/src/std/LegacyBios.h
+++ b/src/std/LegacyBios.h
@@ -228,6 +228,26 @@ typedef struct {
/// Maximum PCI bus number assigned.
///
UINT8 LastPciBus;
+
+ ///
+ /// Start address of UMB RAM
+ ///
+ UINT32UmaAddress;
+
+ ///
+ /// Size of UMB RAM
+ ///
+ UINT32UmaSize;
+
+ ///
+ /// Start address of persistent allocation in high (1MiB) memory
+ ///
+ UINT32HiPermanentMemoryAddress;
+
+ ///
+ /// Size of persistent allocation in high (1MiB) memory
+ ///
+ UINT32HiPermanentMemorySize;
} EFI_COMPATIBILITY16_TABLE;
///
--
1.9.0
--
dwmw2
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [RFC PATCH] Add support for Intel Quark UART.
On Fri, 2013-11-29 at 12:44 -0500, Kevin O'Connor wrote: Do you need debug output from 16bit mode or 32bit segmented mode? The post and boot phases are all 32bit code so typical boot time debugging shouldn't be impacted. Gerd's cbmem debugging code uses this approach. I don't need debug output. But an INT 10h implementation like sgabios¹ would be really useful on Quark, because there's normally no real VGA output (unless you can connect mini-PCI one, which is what I've done so far). However, I note that sgabios already talks about having support for SMM traps to talk to an EFI console, so perhaps that's the way forward. The SMM side can be provided either by UEFI in the CONFIG_CSM case, or by SeaBIOS itself when it's native. You've already been looking at something very similar to this anyway, right? I can live with that. Perhaps I should make it work in 16-bit mode but *only* if the appropriate BAR has been put in a memory hole below 1MiB. That should be okay, but would it ever actually be mapped below 1Meg? Where would it be mapped to: 0xa-0xc? Hm, I suspect we *could* set up the A and B segments to map to MMIO space and then set the BAR for the appropriate UART to be there. But an sgabios implementation is going to need *memory* in the A segment, and play tricks to spot when the application/OS has written there and output the appropriate changes to the serial port. So I'm not sure it's stunningly useful to do so. -- dwmw2 ¹ https://code.google.com/p/sgabios/ smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [RFC PATCH] Add support for Intel Quark UART.
On Mon, 2014-05-19 at 11:20 -0400, Kevin O'Connor wrote: On Mon, May 19, 2014 at 02:27:38PM +0100, David Woodhouse wrote: On Fri, 2013-11-29 at 12:44 -0500, Kevin O'Connor wrote: Do you need debug output from 16bit mode or 32bit segmented mode? The post and boot phases are all 32bit code so typical boot time debugging shouldn't be impacted. Gerd's cbmem debugging code uses this approach. I don't need debug output. But an INT 10h implementation like sgabios¹ would be really useful on Quark, because there's normally no real VGA output (unless you can connect mini-PCI one, which is what I've done so far). Yes, a serial console using mmio serial ports would be a good reason to integrate sgabios functionality into seabios. A couple of months back I looked through the sgabios assembler. It looks mostly straight-forward. The only thing that was marginally complex was its ability to keep a cache of background attributes. However, I note that sgabios already talks about having support for SMM traps to talk to an EFI console, so perhaps that's the way forward. I wasn't aware of that. Do you have a link? https://code.google.com/p/sgabios/source/browse/trunk/design.txt#219 Optionally the serial port input/output can be replaced with a SMI trigger that calls into an EFI BIOS in order to tie into its own console input and output routines rather than directly hitting the serial port. In this particular case it's assumed that all logging is handled in the EFI module that will be called. BIOS int 15h, ax = 0d042h is used to trigger SMI. The parameters passed will need to be changed to be specific to the EFI or SMI handler put in place. In the example in SMBIOS, for output, ebx = 0xf00d | (char 8), and for input, ebx = 0xfeed, with the character, if any, returned in the eax register with ZF set and eax=0 if no character was available. This appears to be a lie. There's nothing but a FIXME in the code (line 849 on the send_byte function). But an sgabios implementation is going to need *memory* in the A segment, and play tricks to spot when the application/OS has written there and output the appropriate changes to the serial port. So I'm not sure it's stunningly useful to do so. sgabios doesn't do anything like that today. Most apps that anyone cares about (ie, modern bootloaders) are well behaving - they output their vga text using the int 10h bios interrupts. Sgabios hooks these interrupts to provide service. In my tests, the only thing I've found which does direct text writes to the 0xa-0xc framebuffer are really old dos programs. Even freedos uses the standard bios interface. Hm, OK. Again I got that from sgabios's design.txt: Known Bugs -- With some versions of DOS, only the last character of every line is displayed once dos boots since DOS will use direct access to the VGA framebuffer until the end of line is reached, at which point it will start using int 10h. Dual cursor tracking might fix this issue by maintaining positions for dos that look like the end of line and another for internal use to know where to output next. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH v2] Update EFI_COMPATIBILITY16_TABLE to match 0.98 spec update
On Fri, 2014-01-10 at 12:49 -0500, Kevin O'Connor wrote: On Fri, Dec 06, 2013 at 11:56:22AM +, David Woodhouse wrote: We expect to use the space between the top of option ROMs and the bottom of our own BIOS code as a stack. OVMF was previously marking the whole region from 0xC to 0xF read-only before invoking our Legacy16Boot method. Read-only stack considered harmful. Version 0.98 of the CSM spec adds the UmaAddress and UmaSize fields, which allow the CSM to specify a memory region that needs to be writable. There exists CONFIG_MALLOC_UPPERMEMORY which we could turn off to use the 9-segment, but that isn't particularly useful for the CSM case either because that memory isn't ours to play with until the final Legacy16Boot call. There's a LowPmmMemory given to use by UEFI to play with, but that's right in the *middle* of low memory and using that for persistent allocations would be painful. So just require CONFIG_MALLOC_UPPERMEMORY when building a CSM. Hi David, Are you still looking at this? If I recall correctly, you were going to run a test without CONFIG_MALLOC_UPPERMEMORY set. Apologies for the sporadic nature of my attention to this. It's back at the top of my pile again for a while. I've submitted the corresponding patch to EDK2 and it should hopefully be accepted now that the spec is published. As I'm testing on Quark today, I tried reverting the EDK2-side patch and the SeaBIOS patch, and disabling CONFIG_MALLOC_UPPERMEMORY. It booted FreeDOS and worked fine. Then I re-enabled CONFIG_MALLOC_UPPERMEMORY and it still worked, which wasn't expected :) I'll have to retest with OVMF under Qemu (with KVM). -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [RFC PATCH] Add support for Intel Quark UART.
On Mon, 2014-05-19 at 13:42 -0400, Kevin O'Connor wrote: I don't see any reference to int 15h, ax=0xd042 as a standard. So, maybe the author of the above text also wrote their own EFI module which used that magic value? Well, the INT 15h call would still be on the BIOS side; that doesn't sound like an EFI thing at all. Perhaps that's a special-case hack to make their platform *trigger* the SMM entry, which then gets handled by the EFI code? Known Bugs -- With some versions of DOS [...] With that predicate, I'm sure any conclusion is true. True. I suppose the question is whether it's true of any versions of DOS that we *care* about. Or other operating systems that we might want to use on Quark, I suppose. Your response seems to suggest not. I just didn't want to design myself into a corner where I'd *never* be able to fix this known bug? -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH 0/7] vgabios improvements
On Wed, 2014-04-16 at 12:37 -0400, Kevin O'Connor wrote: On Mon, Apr 14, 2014 at 02:22:51PM +0200, Gerd Hoffmann wrote: So you can try this: qemu -vga std -bios /usr/share/coreboot.git/coreboot-i440fx-seabios.rom to see it live in action. Two problems spotted so far: (1) ipxe hangs at rom load time. can be worked around by adding '-net none' to the qemu cmd line. I've reproduced this. It only fails for me with -enable-kvm. It was (as I feared) the result of segment limits getting trashed from the int 1587 call. (Presumably, qemu tcg doesn't fail because it doesn't implement segment limits?) The (incorrect) patch below enables the boot to proceed past the ipxe prompt. This is unfortunate. I can put a hack into seabios (not seavgabios) to use bigreal mode for int 1587 calls during option rom execution. But, it does raise the question of how many other callers expect the bios to not mess with the segment limits. (Though, to be honest, the only goal I have with coreboot native seavgabios is support for grub, lilo, syslinux, and maybe ntldr.) Was there a conclusion to this? Did you eventually convince yourself that it could be *OK* for the video BIOS to trash the segment limits, for a limited set of boot targets? -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH v2] Update EFI_COMPATIBILITY16_TABLE to match 0.98 spec update
On Fri, 2014-01-10 at 12:49 -0500, Kevin O'Connor wrote: On Fri, Dec 06, 2013 at 11:56:22AM +, David Woodhouse wrote: We expect to use the space between the top of option ROMs and the bottom of our own BIOS code as a stack. OVMF was previously marking the whole region from 0xC to 0xF read-only before invoking our Legacy16Boot method. Read-only stack considered harmful. Version 0.98 of the CSM spec adds the UmaAddress and UmaSize fields, which allow the CSM to specify a memory region that needs to be writable. There exists CONFIG_MALLOC_UPPERMEMORY which we could turn off to use the 9-segment, but that isn't particularly useful for the CSM case either because that memory isn't ours to play with until the final Legacy16Boot call. There's a LowPmmMemory given to use by UEFI to play with, but that's right in the *middle* of low memory and using that for persistent allocations would be painful. So just require CONFIG_MALLOC_UPPERMEMORY when building a CSM. Hi David, Are you still looking at this? If I recall correctly, you were going to run a test without CONFIG_MALLOC_UPPERMEMORY set. That appears to make it boot OK, although I'm still not sure it's *correct* to be using the 9-segment from the CSM before we've actually been told to *boot*. Maybe we just get lucky. Do you want a version of the patch which doesn't add the dependency on CONFIG_MALLOC_UPPERMEMORY? -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [RFC] SeaBIOS v2.0 and 32bit drivers
On Thu, 2014-04-24 at 22:02 -0400, Kevin O'Connor wrote: I've been considering a possible architectural change to SeaBIOS. Currently, SeaBIOS contains a mix of 16bit code and 32bit code. All of the initialization and bootup code is done in regular 32bit mode, but runtime code (the callbacks the OS uses) is generally run in 16bit mode. I have been thinking about possibly changing this so that hardware driver code runs exclusively in 32bit mode. ... The biggest downside to this change would be problems running old DOS era programs that attempt to run the BIOS in vm86 mode. Specifically, the dos emm386 program is known to prevent 32 bit trampolines in SeaBIOS from working. (There's been a bit of experience with AHCI drivers running in 32bit mode (and now XHCI) so we have good confidence that modern OSes wont present a problem.) To continue to support these old DOS era programs I'm proposing we implement an SMI to help trampoline to 32bit mode. This sounds broadly similar to the approach taken by some CSM implementations. Instead of having native device drivers in the CSM and actually taking over the hardware as SeaBIOS does, they use SMI to trampoline back into UEFI-side code and use the drivers there. If we're going to do this, it would be interesting to see if we can use the same protocol — both for thunking from 16-bit to 32-bit SeaBIOS code, and for thunking from the SeaBIOS CSM to OVMF/UEFI. Using the native UEFI drivers via such a thunk would resolve a number of the issues around who owns the hardware, and how we continue with a UEFI boot after a 'Legacy' boot attempt has failed (to detect the 0x55aa signature on a boot sector, for example). -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [RFC] SeaBIOS v2.0 and 32bit drivers
On Tue, 2014-05-13 at 12:45 -0400, Kevin O'Connor wrote: That is interesting. Do you have a link to code or a spec handy? (I looked through the Compatibility Support Module Specification again and I see it talks about SMMs, but it seems to say that what is implemented is IBV specific.) Right. The CSM specification is fairly incomplete, and large parts of the details around how you enumerate boot targets and communicate that to the CSM are left to the implementer. It's only on chasing up my questions about how it's supposed to work, that I ended up being told that we expect it all to happen with SMM and native UEFI drivers, and the CSM really having very little hardware-specific code at all. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] [PATCH v2] Update EFI_COMPATIBILITY16_TABLE to match 0.98 spec update
We expect to use the space between the top of option ROMs and the bottom
of our own BIOS code as a stack. OVMF was previously marking the whole
region from 0xC to 0xF read-only before invoking our Legacy16Boot
method. Read-only stack considered harmful.
Version 0.98 of the CSM spec adds the UmaAddress and UmaSize fields, which
allow the CSM to specify a memory region that needs to be writable.
There exists CONFIG_MALLOC_UPPERMEMORY which we could turn off to use
the 9-segment, but that isn't particularly useful for the CSM case
either because that memory isn't ours to play with until the final
Legacy16Boot call. There's a LowPmmMemory given to use by UEFI to play
with, but that's right in the *middle* of low memory and using that for
persistent allocations would be painful. So just require
CONFIG_MALLOC_UPPERMEMORY when building a CSM.
Signed-off-by: David Woodhouse david.woodho...@intel.com
---
v2: Require CONFIG_MALLOC_UPPERMEMORY. Default UmaAddress to zonelow_base.
src/Kconfig | 2 +-
src/fw/csm.c | 6 +-
src/std/LegacyBios.h | 20
3 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/src/Kconfig b/src/Kconfig
index a42ab2d..093075c 100644
--- a/src/Kconfig
+++ b/src/Kconfig
@@ -114,7 +114,7 @@ endchoice
the BIOS in 16bit protected mode.
config MALLOC_UPPERMEMORY
-bool Allocate memory that needs to be in first Meg above 0xc
+bool Allocate memory that needs to be in first Meg above 0xc if
!CSM
default y
help
Use the Upper Memory Block area (0xc-0xf) for
diff --git a/src/fw/csm.c b/src/fw/csm.c
index dfb0d12..4e4b688 100644
--- a/src/fw/csm.c
+++ b/src/fw/csm.c
@@ -34,6 +34,8 @@ EFI_COMPATIBILITY16_TABLE csm_compat_table VARFSEG
__aligned(16) = {
.Compatibility16CallOffset = 0 /* Filled in by checkrom.py */,
.OemIdStringPointer = (u32)SeaBIOS,
.AcpiRsdPtrPointer = (u32)csm_rsdp,
+.UmaAddress = (u32)zonelow_base,
+.UmaSize = 0x1,
};
EFI_TO_COMPATIBILITY16_INIT_TABLE *csm_init_table;
@@ -46,9 +48,11 @@ extern void __csm_return(struct bregs *regs) __noreturn;
static void
csm_return(struct bregs *regs)
{
-dprintf(3, handle_csm returning AX=%04x\n, regs-ax);
+u32 top = rom_get_max();
PICMask = pic_irqmask_read();
+csm_compat_table.UmaAddress = top;
+csm_compat_table.UmaSize = 0xf - top;
__csm_return(regs);
}
diff --git a/src/std/LegacyBios.h b/src/std/LegacyBios.h
index cf0c3c5..5170c37 100644
--- a/src/std/LegacyBios.h
+++ b/src/std/LegacyBios.h
@@ -228,6 +228,26 @@ typedef struct {
/// Maximum PCI bus number assigned.
///
UINT8 LastPciBus;
+
+ ///
+ /// Start address of UMB RAM
+ ///
+ UINT32UmaAddress;
+
+ ///
+ /// Size of UMB RAM
+ ///
+ UINT32UmaSize;
+
+ ///
+ /// Start address of persistent allocation in high (1MiB) memory
+ ///
+ UINT32HiPermanentMemoryAddress;
+
+ ///
+ /// Size of persistent allocation in high (1MiB) memory
+ ///
+ UINT32HiPermanentMemorySize;
} EFI_COMPATIBILITY16_TABLE;
///
--
1.8.3.1
--
David WoodhouseOpen Source Technology Centre
david.woodho...@intel.com Intel Corporation
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] [PATCH 1/3] Introduce serial_out(), serial_in() macros in serialio.c
This will make it easier to abstract out the MMIO serial access. Even if that ends up in separate code which is a *copy* of this, at least they'll match. Signed-off-by: David Woodhouse david.woodho...@intel.com --- src/hw/serialio.c | 19 +++ 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/hw/serialio.c b/src/hw/serialio.c index 6486fc0..8ec36c2 100644 --- a/src/hw/serialio.c +++ b/src/hw/serialio.c @@ -1,6 +1,6 @@ // Low-level serial (and serial-like) device access. // -// Copyright (C) 2008-1013 Kevin O'Connor ke...@koconnor.net +// Copyright (C) 2008-2013 Kevin O'Connor ke...@koconnor.net // // This file may be distributed under the terms of the GNU LGPLv3 license. @@ -17,6 +17,9 @@ #define DEBUG_TIMEOUT 10 +#define serial_out(d, reg) outb(d, CONFIG_DEBUG_SERIAL_PORT+(reg)) +#define serial_in(reg) inb(CONFIG_DEBUG_SERIAL_PORT+(reg)) + // Setup the debug serial port for output. void serial_debug_preinit(void) @@ -25,12 +28,12 @@ serial_debug_preinit(void) return; // setup for serial logging: 8N1 u8 oldparam, newparam = 0x03; -oldparam = inb(CONFIG_DEBUG_SERIAL_PORT+SEROFF_LCR); -outb(newparam, CONFIG_DEBUG_SERIAL_PORT+SEROFF_LCR); +oldparam = serial_in(SEROFF_LCR); +serial_out(newparam, SEROFF_LCR); // Disable irqs u8 oldier, newier = 0; -oldier = inb(CONFIG_DEBUG_SERIAL_PORT+SEROFF_IER); -outb(newier, CONFIG_DEBUG_SERIAL_PORT+SEROFF_IER); +oldier = serial_in(SEROFF_IER); +serial_out(newier, SEROFF_IER); if (oldparam != newparam || oldier != newier) dprintf(1, Changing serial settings was %x/%x now %x/%x\n @@ -44,11 +47,11 @@ serial_debug(char c) if (!CONFIG_DEBUG_SERIAL) return; int timeout = DEBUG_TIMEOUT; -while ((inb(CONFIG_DEBUG_SERIAL_PORT+SEROFF_LSR) 0x20) != 0x20) +while ((serial_in(SEROFF_LSR) 0x20) != 0x20) if (!timeout--) // Ran out of time. return; -outb(c, CONFIG_DEBUG_SERIAL_PORT+SEROFF_DATA); +serial_out(c, SEROFF_DATA); } void @@ -66,7 +69,7 @@ serial_debug_flush(void) if (!CONFIG_DEBUG_SERIAL) return; int timeout = DEBUG_TIMEOUT; -while ((inb(CONFIG_DEBUG_SERIAL_PORT+SEROFF_LSR) 0x60) != 0x60) +while ((serial_in(SEROFF_LSR) 0x60) != 0x60) if (!timeout--) // Ran out of time. return; -- 1.8.3.1 -- David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] [PATCH 2/3] Add Intel Quark UART support
This includes generic support for MMIO serial which can be used for
other PCI (or non-PCI) serial ports too.
Only outputs to the serial port in 32-bit mode, unless we're lucky
enough that it's at an address below 1MiB.
Signed-off-by: David Woodhouse david.woodho...@intel.com
---
src/Kconfig | 15 ++-
src/hw/serialio.c | 56 +++
2 files changed, 70 insertions(+), 1 deletion(-)
diff --git a/src/Kconfig b/src/Kconfig
index 92c3102..696ab46 100644
--- a/src/Kconfig
+++ b/src/Kconfig
@@ -477,8 +477,21 @@ menu Debugging
default n
help
Send debugging information to serial port.
-config DEBUG_SERIAL_PORT
+config DEBUG_SERIAL_MMIO
depends on DEBUG_SERIAL
+bool
+default n
+help
+Send debugging information to PCI serial port.
+config DEBUG_SERIAL_QUARK
+bool Intel Quark serial port debugging
+ depends on DEBUG_SERIAL
+select DEBUG_SERIAL_MMIO
+default n
+help
+Use Quark UART1 (PCI device 0x14, function 5) for serial debug.
+config DEBUG_SERIAL_PORT
+depends on DEBUG_SERIAL !DEBUG_SERIAL_MMIO
hex Serial port base address
default 0x3f8
help
diff --git a/src/hw/serialio.c b/src/hw/serialio.c
index 8ec36c2..b2bc68f 100644
--- a/src/hw/serialio.c
+++ b/src/hw/serialio.c
@@ -9,6 +9,9 @@
#include output.h // dprintf
#include serialio.h // serial_debug_preinit
#include x86.h // outb
+#include hw/pci_regs.h // PCI_VENDOR_ID, PCI_BASE_ADDRESS_0
+#include hw/pci.h // pci_config_readl
+#include stacks.h // call32
/
@@ -17,8 +20,42 @@
#define DEBUG_TIMEOUT 10
+#if CONFIG_DEBUG_SERIAL_MMIO
+static u32 serial_addr;
+static u8 serial_stride;
+#define serial_out(d, ofs) writeb((void *)(serial_addr + (ofs)*serial_stride),
(d))
+#define serial_in(ofs) readb((void *)(serial_addr + (ofs)*serial_stride))
+// Debug output only in 32-bit mode, or when MMIO address 1MiB
+#define serial_valid() (serial_addr (!MODE16 || serial_addr 0x10))
+#define set_serial_addr(adr, str) do { serial_addr = (adr); \
+ serial_stride = (str); } while (0)
+#else
#define serial_out(d, reg) outb(d, CONFIG_DEBUG_SERIAL_PORT+(reg))
#define serial_in(reg) inb(CONFIG_DEBUG_SERIAL_PORT+(reg))
+#define serial_valid() (1)
+#define set_serial_addr(adr, str) do { ; } while (0)
+#endif
+
+static int
+find_mmio_serial(void)
+{
+if (CONFIG_DEBUG_SERIAL_QUARK) {
+// debug port is bus 0, device 0x14, function 5.
+u16 uart_bdf = pci_to_bdf(0, 0x14, 5);
+
+// If it isn't a Quark UART...
+if (pci_config_readl(uart_bdf, PCI_VENDOR_ID) != 0x09368086)
+return -1;
+
+u32 bar0 = pci_config_readl(uart_bdf, PCI_BASE_ADDRESS_0);
+if (!(bar0 0xf)) {
+set_serial_addr(bar0, 4);
+return 0;
+}
+}
+
+return -1;
+}
// Setup the debug serial port for output.
void
@@ -26,6 +63,21 @@ serial_debug_preinit(void)
{
if (!CONFIG_DEBUG_SERIAL)
return;
+
+if (CONFIG_DEBUG_SERIAL_MMIO) {
+// Need to discover serial port MMIO address
+if (MODE16) {
+// Needs to be done in 32-bit mode.
+extern void _cfunc32flat_serial_debug_preinit(void);
+call32(_cfunc32flat_serial_debug_preinit, 0, -1);
+return;
+}
+
+// Find the PCI device or whatever provides the serial port
+if (find_mmio_serial())
+return;
+}
+
// setup for serial logging: 8N1
u8 oldparam, newparam = 0x03;
oldparam = serial_in(SEROFF_LCR);
@@ -46,6 +98,8 @@ serial_debug(char c)
{
if (!CONFIG_DEBUG_SERIAL)
return;
+if (!serial_valid())
+return;
int timeout = DEBUG_TIMEOUT;
while ((serial_in(SEROFF_LSR) 0x20) != 0x20)
if (!timeout--)
@@ -68,6 +122,8 @@ serial_debug_flush(void)
{
if (!CONFIG_DEBUG_SERIAL)
return;
+if (!serial_valid())
+return;
int timeout = DEBUG_TIMEOUT;
while ((serial_in(SEROFF_LSR) 0x60) != 0x60)
if (!timeout--)
--
1.8.3.1
--
David WoodhouseOpen Source Technology Centre
david.woodho...@intel.com Intel Corporation
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] [PATCH 3/3] Initialise debug output in handle_csm()
For a PCI serial port, we have to discover its location. Potentially on every entry, since UEFI may shift it at some point during the boot process from an early debug setup to a final location after real PCI enumeration. Signed-off-by: David Woodhouse david.woodho...@intel.com --- src/fw/csm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/fw/csm.c b/src/fw/csm.c index 4e4b688..39634cd 100644 --- a/src/fw/csm.c +++ b/src/fw/csm.c @@ -109,6 +109,8 @@ handle_csm_0001(struct bregs *regs) return; } +debug_preinit(); + dprintf(3, Legacy16UpdateBbs table %04x:%04x\n, regs-es, regs-bx); csm_boot_table = MAKE_FLATPTR(regs-es, regs-bx); -- 1.8.3.1 -- David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] 'make oldconfig' broken...
Why does it keep forgetting my config and reverting to defaults, every time I touch src/Kconfig? [dwmw2@shinybook seabios]$ make oldconfig scripts/kconfig/conf --oldconfig /home/dwmw2/git/seabios/src/Kconfig # # configuration written to /home/dwmw2/git/seabios/.config # [dwmw2@shinybook seabios]$ grep CSM .config CONFIG_CSM=y [dwmw2@shinybook seabios]$ touch src/Kconfig [dwmw2@shinybook seabios]$ make oldconfig Build default config # # configuration written to /home/dwmw2/git/seabios/.config # scripts/kconfig/conf --oldconfig /home/dwmw2/git/seabios/src/Kconfig # # configuration written to /home/dwmw2/git/seabios/.config # [dwmw2@shinybook seabios]$ grep CSM .config # CONFIG_CSM is not set [dwmw2@shinybook seabios]$ -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] USB1 devices and CONFIG_THREADS
Just debugged the following: ehci_setup() happens. Starts a thread to probe its ports... ohci_setup() happens. Starts a thread to probe its ports... OHCI probes port zero, decides there's nothing there. EHCI probes port zero, decides there's a USB1 device there and gives it away to OHCI... which is never going to see it. What's wrong here? I've fixed it with a wait_threads() right before the ohci_setup() in usb.c and now I can see my keyboard, but that's definitely not the right answer. Do we have any kind of hotplug support for USB devices? If not, should we at least have a kind of manual hotplug notification for when EHCI actively gives a port away to the USB1 controller? (How) is this supposed to work? -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] USB1 devices and CONFIG_THREADS
On Thu, 2013-12-05 at 16:11 +0100, Gerd Hoffmann wrote: I think not running configure_ehci() as thread should work too and is a bit less agressive than wait_threads(). Waits only for the ehci port probe threads finish (see usb_enumerate() func), not all threads. Yeah. Is EHCI always initialised before OHCI/UHCI though? I'm staring at the code in usb_setup() and it seems to be attempting that, but I'm not 100% sure if that's what it's actually doing. And what about XHCI? Does that have similar interactions? -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] USB1 devices and CONFIG_THREADS
On Thu, 2013-12-05 at 11:59 -0500, Kevin O'Connor wrote: I reviewed the code and booted my e350m1 board with an OHCI keyboard and I can't reproduce your issue. Can you post the logs (with debug level 8) of the failure you are seeing? With some debugging of my own added. Note that ohci_setup() is called from usb_setup(). Not from ehci_note_port(). And that this happens: |00fe1000| ohci_hub_detect port 0: sts 0 Before this does: |00fe3000| port 0 has a full speed device |00fe3000| ehci_hub_reset returns -1 I think this is because the EHCI device has a lower PCI function (0:14.3) than its corresponding OHCI device (0:14.4)? Trying to make sense of the loop in usb_setup(), I think it only really works if the EHCI device comes *after* the OHCI device? [dwmw2@shinybook Quark_EDKII_0_8_0_RC3_1]$ grep -A103 'init usb' ~/git/clantonusb5.cap | sed 's/ *$//' init usb ehci_setup() _malloc zone=0x00fefe8f handle= size=72 align=10 ret=0x00fe5e60 (detail=0x00fe5eb0) EHCI init on dev 00:14.3 (regs=0x9000d010) _malloc zone=0x00fefe8f handle= size=4096 align=1000 ret=0x00fe4000 (detail=0x00fe5e30) /00fe4000\ Start thread |00fe4000| _malloc zone=0x00fefe9b handle= size=4096 align=1000 ret=0x00fff000 (detail=0x00fe5e00) |00fe4000| _malloc zone=0x00fefe9b handle= size=68 align=80 ret=0x00ffef80 (detail=0x00fe5dd0) |00fe4000| _malloc zone=0x00fefe9b handle= size=68 align=80 ret=0x00ffef00 (detail=0x00fe5da0) |00fe4000| _malloc zone=0x00fefe8f handle= size=28 align=10 ret=0x00fe5d50 (detail=0x00fe5d70) |00fe4000| _malloc zone=0x00fefe8f handle= size=4096 align=1000 ret=0x00fe3000 (detail=0x00fe5d20) /00fe3000\ Start thread ohci_setup() called from usb_setup ohci_setup() _malloc zone=0x00fefe8f handle= size=24 align=10 ret=0x00fe5cd0 (detail=0x00fe5cf0) OHCI init on dev 00:14.4 (regs=0x9000c000) _malloc zone=0x00fefe8f handle= size=4096 align=1000 ret=0x00fe2000 (detail=0x00fe5ca0) /00fe2000\ Start thread |00fe2000| _malloc zone=0x00fefe9b handle= size=256 align=100 ret=0x00ffee00 (detail=0x00fe5c70) |00fe2000| _malloc zone=0x00fefe9b handle= size=16 align=10 ret=0x00ffeff0 (detail=0x00fe5c40) |00fe4000| _malloc zone=0x00fefe8f handle= size=28 align=10 ret=0x00fe5bf0 (detail=0x00fe5c10) |00fe4000| _malloc zone=0x00fefe8f handle= size=4096 align=1000 ret=0x00fe1000 (detail=0x00fe5bc0) /00fe1000\ Start thread init ps2port _malloc zone=0x00fefe8f handle= size=4096 align=1000 ret=0x00fe (detail=0x00fe5b90) /00fe\ Start thread |00fe| i8042_flush |00fe| i8042 flushed ff (status=ff) |00fe| i8042 flushed ff (status=ff) |00fe| i8042 flushed ff (status=ff) |00fe| i8042 flushed ff (status=ff) |00fe| i8042 flushed ff (status=ff) |00fe| i8042 flushed ff (status=ff) |00fe| i8042 flushed ff (status=ff) |00fe| i8042 flushed ff (status=ff) |00fe| i8042 flushed ff (status=ff) |00fe| i8042 flushed ff (status=ff) |00fe| i8042 flushed ff (status=ff) |00fe| i8042 flushed ff (status=ff) |00fe| i8042 flushed ff (status=ff) |00fe| i8042 flushed ff (status=ff) |00fe| i8042 flushed ff (status=ff) |00fe| i8042 flushed ff (status=ff) |00fe| WARNING - Timeout at i8042_flush:71! |00fe2000| _free 0x00fe (detail=0x00fe5b90) \00fe/ End thread |00fe1000| ehci_hub_detect 1 not found one |00fe1000| _free 0x00fe5bf0 (detail=0x00fe5c10) |00fe3000| _free 0x00fe1000 (detail=0x00fe5bc0) \00fe1000/ End thread |00fe3000| ehci_hub_detect 0 found one init lpt Found 0 lpt ports init serial Found 0 serial ports init floppy drives init hard drives init ahci Found floppy of size 1474560 _malloc zone=0x00fefe97 handle= size=36 align=10 ret=0x000f5720 (detail=0x00fe5c10) Mapping SPI floppy at addr 0xff80 _malloc zone=0x00fefe8f handle= size=24 align=10 ret=0x00fe5bc0 (detail=0x00fe5be0) Registering bootable: Ramdisk [SPI] (type:1 prio:0 data:f5720) init megasas handle_08 handle_hwpic1 irq=1 |00fe2000| _malloc zone=0x00fefe8f handle= size=28 align=10 ret=0x00fe5b70 (detail=0x00fe5b90) |00fe2000| _malloc zone=0x00fefe8f handle= size=4096 align=1000 ret=0x00fe1000 (detail=0x00fe5b40) /00fe1000\ Start thread |00fe1000| ohci_hub_detect port 0: sts 0 |00fe1000| _free 0x00fe5b70 (detail=0x00fe5b90) |00fe4000| _free 0x00fe1000 (detail=0x00fe5b40) \00fe1000/ End thread |00fe3000| port 0 has a full speed device |00fe3000| ehci_hub_reset returns -1 |00fe3000| port 0 speed -1 |00fe3000| _free 0x00fe5d50 (detail=0x00fe5d70) _free 0x00fe3000 (detail=0x00fe5d20) \00fe3000/ End thread handle_08 |00fe2000| _malloc zone=0x00fefe8f handle= size=28 align=10 ret=0x00fe5d50 (detail=0x00fe5d70) |00fe2000| _malloc zone=0x00fefe8f handle= size=4096 align=1000 ret=0x00fe3000 (detail=0x00fe5d20) /00fe3000\ Start thread |00fe3000| ohci_hub_detect port 1: sts 0 |00fe3000| _free 0x00fe5d50 (detail=0x00fe5d70) |00fe4000| _free
Re: [SeaBIOS] USB1 devices and CONFIG_THREADS
On Thu, 2013-12-05 at 21:26 +, David Woodhouse wrote: I think this is because the EHCI device has a lower PCI function (0:14.3) than its corresponding OHCI device (0:14.4)? It is indeed. And reading the EHCI spec, I see that the companion controllers *must* have a lower function number than the EHCI. This appears to be a hardware bug. Will chase internally, but may need a quirk for this. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] USB1 devices and CONFIG_THREADS
On Thu, 2013-12-05 at 18:59 -0500, Kevin O'Connor wrote:
It occurred to me that we could actually simplify the code by
initializing all the ehci controllers and then init all the ohci/uhci
controllers. The current complex interactions between the PCI walking
and EHCI setup is really hard to understand. I put together a sample
patch - see attached and at:
https://github.com/KevinOConnor/seabios/tree/testing
That works here, and I think I prefer it to what I'd come up with:
diff --git a/src/hw/usb-ehci.c b/src/hw/usb-ehci.c
index b495d6c..e800322 100644
--- a/src/hw/usb-ehci.c
+++ b/src/hw/usb-ehci.c
@@ -371,6 +371,9 @@ ehci_setup(struct pci_device *pci, int busid, struct
pci_device *comppci)
cntl-companion[count++] = comppci;
else if (pci_classprog(comppci) == PCI_CLASS_SERIAL_USB_OHCI)
cntl-companion[count++] = comppci;
+// Intel Quark quirk. Precisely one OHCI comes *after* the EHCI.
+if (comppci-bdf pci-bdf)
+break;
comppci = container_of(comppci-node.next, struct pci_device, node);
}
diff --git a/src/hw/usb.c b/src/hw/usb.c
index 8fe741f..9c09e42 100644
--- a/src/hw/usb.c
+++ b/src/hw/usb.c
@@ -461,12 +461,28 @@ usb_setup(void)
for (;;) {
if (pci_classprog(ehcipci) == PCI_CLASS_SERIAL_USB_EHCI) {
// Found an ehci controller.
+
+// Intel Quark got EHCI and OHCI the wrong way round :(
+if (pci-vendor == PCI_VENDOR_ID_INTEL
+pci-device == 0x0939) {
+struct pci_device *nextpci;
+nextpci = container_of_or_null(ehcipci-node.next,
+ struct pci_device,
node);
+if (nextpci nextpci-vendor == PCI_VENDOR_ID_INTEL
+nextpci-device == 0x093a
+pci_bdf_to_busdev(nextpci-bdf) ==
+ pci_bdf_to_busdev(pci-bdf))
{
+pci = nextpci;
+found++;
+}
+}
int ret = ehci_setup(ehcipci, count++, pci);
if (ret)
// Error
break;
count += found;
-pci = ehcipci;
+if (pci-bdf ehcipci-bdf)
+pci = ehcipci;
break;
}
if (ehcipci-class == PCI_CLASS_SERIAL_USB)
@@ -479,6 +495,9 @@ usb_setup(void)
break;
}
}
+if (pci-bdf ehcipci-bdf)
+// Quark OHCI. It's been handled.
+continue;
if (pci_classprog(pci) == PCI_CLASS_SERIAL_USB_UHCI)
uhci_setup(pci, count++);
--
dwmw2
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] [PATCH] Update EFI_COMPATIBILITY16_TABLE to match 0.98 spec update
We expect to use the space between the top of option ROMs and the bottom
of our own BIOS code as a stack. OVMF was previously marking the whole
region from 0xC to 0xF read-only before invoking our Legacy16Boot
method. Read-only stack considered harmful.
Version 0.98 of the CSM spec adds the UmaAddress and UmaSize fields, which
allow the CSM to specify a memory region that needs to be writable.
Signed-off-by: David Woodhouse david.woodho...@intel.com
---
The corresponding patch hasn't hit the OVMF/EDKII tree yet, but that
shouldn't stop us from applying it anyway. CSM support was completely
broken before this anyway — it required *some* way of hacking around
this problem. And this patch does not harm on its own either.
src/fw/csm.c | 6 +-
src/std/LegacyBios.h | 20
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/src/fw/csm.c b/src/fw/csm.c
index afd7ffe..6fe3a1b 100644
--- a/src/fw/csm.c
+++ b/src/fw/csm.c
@@ -34,6 +34,8 @@ EFI_COMPATIBILITY16_TABLE csm_compat_table VARFSEG
__aligned(16) = {
.Compatibility16CallOffset = 0 /* Filled in by checkrom.py */,
.OemIdStringPointer = (u32)SeaBIOS,
.AcpiRsdPtrPointer = (u32)csm_rsdp,
+.UmaAddress = 0xe,
+.UmaSize = 0x1,
};
EFI_TO_COMPATIBILITY16_INIT_TABLE *csm_init_table;
@@ -46,9 +48,11 @@ extern void __csm_return(struct bregs *regs) __noreturn;
static void
csm_return(struct bregs *regs)
{
-dprintf(3, handle_csm returning AX=%04x\n, regs-ax);
+u32 top = rom_get_max();
PICMask = pic_irqmask_read();
+csm_compat_table.UmaAddress = top;
+csm_compat_table.UmaSize = 0xf - top;
__csm_return(regs);
}
diff --git a/src/std/LegacyBios.h b/src/std/LegacyBios.h
index cf0c3c5..5170c37 100644
--- a/src/std/LegacyBios.h
+++ b/src/std/LegacyBios.h
@@ -228,6 +228,26 @@ typedef struct {
/// Maximum PCI bus number assigned.
///
UINT8 LastPciBus;
+
+ ///
+ /// Start address of UMB RAM
+ ///
+ UINT32UmaAddress;
+
+ ///
+ /// Size of UMB RAM
+ ///
+ UINT32UmaSize;
+
+ ///
+ /// Start address of persistent allocation in high (1MiB) memory
+ ///
+ UINT32HiPermanentMemoryAddress;
+
+ ///
+ /// Size of persistent allocation in high (1MiB) memory
+ ///
+ UINT32HiPermanentMemorySize;
} EFI_COMPATIBILITY16_TABLE;
///
--
1.8.3.1
--
David WoodhouseOpen Source Technology Centre
david.woodho...@intel.com Intel Corporation
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] [RFC PATCH] Add support for Intel Quark UART.
This provides basic debug output on the Quark system, assuming that
*something* (i.e. coreboot or UEFI) has set it up in advance for us.
Signed-off-by: David Woodhouse david.woodho...@intel.com
---
I looked briefly at making this part of the CONFIG_DEBUG_SERIAL code,
and making that generic enough to handle I/O access *or* MMIO access
depending on what's present... but in fact that's probably overkill.
This isn't really limited to Quark; it would work with any 16550 device
wired up as MMIO32. But we can expand it as required, I think. No point
in starting off with the same functionality as the 5000-odd lines of the
Linux kernel's 8250_pci.c.
What do I need to do if called in 32-bit segmented mode? I'm guessing
that's not going to work right now...
src/Kconfig | 5 +
src/fw/csm.c | 2 ++
src/output.c | 64
3 files changed, 71 insertions(+)
diff --git a/src/Kconfig b/src/Kconfig
index a42ab2d..bdc2602 100644
--- a/src/Kconfig
+++ b/src/Kconfig
@@ -472,6 +472,11 @@ menu Debugging
Set to zero to disable debugging.
+config DEBUG_QUARK_UART
+depends on DEBUG_LEVEL != 0
+bool Debug to Quark UART #1
+default n
+
config DEBUG_SERIAL
depends on DEBUG_LEVEL != 0
bool Serial port debugging
diff --git a/src/fw/csm.c b/src/fw/csm.c
index dfb0d12..afd7ffe 100644
--- a/src/fw/csm.c
+++ b/src/fw/csm.c
@@ -280,6 +280,8 @@ handle_csm(struct bregs *regs)
if (!CONFIG_CSM)
return;
+debug_serial_preinit();
+
dprintf(3, handle_csm regs %p AX=%04x\n, regs, regs-ax);
pic_irqmask_write(PICMask);
diff --git a/src/output.c b/src/output.c
index b47625f..a90d350 100644
--- a/src/output.c
+++ b/src/output.c
@@ -17,6 +17,8 @@
#include stacks.h // call16_int
#include string.h // memset
#include util.h // ScreenAndDebug
+#include hw/pci_regs.h // PCI_VENDOR_ID, PCI_BASE_ADDRESS_0
+#include hw/pci.h // pci_config_readl
struct putcinfo {
void (*func)(struct putcinfo *info, char c);
@@ -31,9 +33,59 @@ struct putcinfo {
u16 DebugOutputPort VARFSEG = 0x402;
+static volatile u32 *quark_uart_addr = 0;
+
+extern void _cfunc32flat_quark_uart_preinit(void);
+extern void _cfunc32flat_quark_uart_putc(void);
+extern void _cfunc32flat_quark_uart_flush(void);
+
+void quark_uart_preinit(void)
+{
+// debug port is bus 0, device 0x14, function 5.
+u16 uart_bdf = pci_to_bdf(0, 0x14, 5);
+
+// If it isn't a Quark UART...
+if (pci_config_readl(uart_bdf, PCI_VENDOR_ID) != 0x09368086)
+return;
+
+u32 bar0 = pci_config_readl(uart_bdf, PCI_BASE_ADDRESS_0);
+if (!(bar0 0xf))
+quark_uart_addr = (void *)bar0;
+}
+
+void quark_uart_putc(char c)
+{
+if (!quark_uart_addr)
+return;
+
+int timeout = DEBUG_TIMEOUT;
+while ((quark_uart_addr[SEROFF_LSR] 0x20) != 0x20)
+if (!timeout--)
+// Ran out of time.
+return;
+quark_uart_addr[SEROFF_DATA] = c;
+}
+
+void quark_uart_flush(void)
+{
+if (!quark_uart_addr)
+return;
+int timeout = DEBUG_TIMEOUT;
+while ((quark_uart_addr[SEROFF_LSR] 0x60) != 0x60)
+if (!timeout--)
+// Ran out of time.
+return;
+}
+
void
debug_serial_preinit(void)
{
+if (CONFIG_DEBUG_QUARK_UART) {
+ if (MODE16)
+ call32(_cfunc32flat_quark_uart_preinit, 0, 0);
+ else
+ quark_uart_preinit();
+}
if (!CONFIG_DEBUG_SERIAL)
return;
// setup for serial logging: 8N1
@@ -54,6 +106,12 @@ debug_serial_preinit(void)
static void
debug_serial(char c)
{
+if (CONFIG_DEBUG_QUARK_UART) {
+ if (MODE16)
+ call32(_cfunc32flat_quark_uart_putc, c, 0);
+ else
+ quark_uart_putc(c);
+}
if (!CONFIG_DEBUG_SERIAL)
return;
int timeout = DEBUG_TIMEOUT;
@@ -68,6 +126,12 @@ debug_serial(char c)
static void
debug_serial_flush(void)
{
+if (CONFIG_DEBUG_QUARK_UART) {
+ if (MODE16)
+ call32(_cfunc32flat_quark_uart_flush, 0, 0);
+ else
+ quark_uart_flush();
+}
if (!CONFIG_DEBUG_SERIAL)
return;
int timeout = DEBUG_TIMEOUT;
--
1.8.3.1
--
dwmw2
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [RFC PATCH] Add support for Intel Quark UART.
On Fri, 2013-11-29 at 11:40 -0500, Kevin O'Connor wrote: On Fri, Nov 29, 2013 at 02:02:02PM +, David Woodhouse wrote: This provides basic debug output on the Quark system, assuming that *something* (i.e. coreboot or UEFI) has set it up in advance for us. Signed-off-by: David Woodhouse david.woodho...@intel.com --- I looked briefly at making this part of the CONFIG_DEBUG_SERIAL code, and making that generic enough to handle I/O access *or* MMIO access depending on what's present... but in fact that's probably overkill. This isn't really limited to Quark; it would work with any 16550 device wired up as MMIO32. But we can expand it as required, I think. No point in starting off with the same functionality as the 5000-odd lines of the Linux kernel's 8250_pci.c. What do I need to do if called in 32-bit segmented mode? I'm guessing that's not going to work right now... Do you need debug output from 16bit mode or 32bit segmented mode? The post and boot phases are all 32bit code so typical boot time debugging shouldn't be impacted. Gerd's cbmem debugging code uses this approach. I can live with that. Perhaps I should make it work in 16-bit mode but *only* if the appropriate BAR has been put in a memory hole below 1MiB. Now I've got the Quark running... has anyone ever looked at sdhci support... ? :) -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [RFC PATCH] Add support for Intel Quark UART.
On Fri, 2013-11-29 at 12:44 -0500, Kevin O'Connor wrote: That should be okay, but would it ever actually be mapped below 1Meg? Where would it be mapped to: 0xa-0xc? Somewhere down there. Or 0xc even. There's no video here. BTW, how does this Quark support compare with the PCI Oxford serial code that Google has been maintaining? http://git.chromium.org/gitweb/?p=chromiumos/third_party/seabios.git;a=commit;h=9b39499125f22627aaeddabfde787c50d51c Hm, close. Mine is mmio32 not mmio8. So IER is at 0x4 in the BAR, IIR at 0x8 etc. And I need to specify the PCI B/D/F because I really need to use the one at :14:5 not the other port at :14.1. But those are easy enough to solve. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] qemu: piix: PCI bridge ACPI hotplug support
On Mon, 2013-06-10 at 19:11 -0500, Anthony Liguori wrote: If we did this right in QEMU, we'd have to introduce a QOM property anyway ... and then we'd have to update each firmware implementation to know about this new property, and the how it translates to the RESET_REG field in the DSDT, etc. So we *still* end up having to update firmwares to keep up with qemu, much more than we would if we'd generated the tables on the qemu side. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] qemu: piix: PCI bridge ACPI hotplug support
On Mon, 2013-06-10 at 18:34 -0500, Anthony Liguori wrote: Internally within QEMU, this initial discussion started by saying that any ACPI generation within QEMU should happen strictly with QOM methods. This was the crux of my argument, if QOM is the only interface we need, everything is there for the firmware to do the same job that QEMU would be doing. That's nice in theory, but I'm not sure how it works as things evolve and new things / new features get exposed. The firmware's *interpretation* of the QOM tree needs to be kept in sync with qemu. Hm, make that: The firmwares' *interpretation*… Let's take a specific, recent example. We fixed the PIIX4 code to actually handle the hard reset on port 0xcf9. We need to fix the ACPI tables to indicate a usable RESET_REG. How is that exposed in the QOM tree, and how does it all work? With qemu exposing ACPI tables in their close-to-final form, it's just fine. Boot with a recent qemu and it's all nice and shiny; boot with an old qemu and it doesn't reset properly. But if the firmware has to be updated to interpret the new feature advertised in the QOM tree and translate it into the ACPI table, then we haven't really got much of an improvement. Please explain how this is supposed to work in *practice*. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] KVM call agenda for 2013-05-28
On Thu, 2013-05-30 at 09:20 -0700, Jordan Justen wrote: On Thu, May 30, 2013 at 5:19 AM, David Woodhouse dw...@infradead.org wrote: On Thu, 2013-05-30 at 13:13 +0200, Laszlo Ersek wrote: Where is CorebootPkg available from? https://github.com/pgeorgi/edk2/tree/coreboot-pkg Is the license on this actually BSD as the License.txt indicates? Is this planned to be upstreamed? Does this support UEFI variables? Does this support UEFI IA32 / X64? Those are questions for Patrick. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] KVM call agenda for 2013-05-28
On Wed, 2013-05-29 at 21:12 -0400, Kevin O'Connor wrote: I remain doubtful that QOM has all the info needed to generate the BIOS tables. Does QOM describe how the 5th pci device uses global interrupt 11 when using global interrupts, legacy interrupt 5 when not using global interrupts, and that the legacy interrupt can be changed by writing to the 0x60 address of the 1st pci device's config space? Does QOM state that the machine supports S3 sleep mode? Does QOM indicate that an IPMI device supports the 3rd version of the IPMI device specification? Does it indicate whether this particular version of qemu has correctly implemented the hard reset at 0xcf9? If so, we need to put that in as the ACPI RESET_REG. It seems that there's a *lot* which isn't fully described in the QOM tree. Do we really want to add it all, just so that ACPI tables can be reliably generated from it? As we add new types of hardware and even fix/adjust features like the examples above, we'll also have to implement the translation from QOM to ACPI tables. And we'll have to do so in more than one place, in projects with a completely different release cycle. This would be *so* much easier if the code which actually generates the ACPI tables was *in* the qemu tree along with the hardware that those tables describe. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] KVM call agenda for 2013-05-28
On Wed, 2013-05-29 at 11:18 -0500, Anthony Liguori wrote: Certainly an option, but that is a long-term project. Out of curiousity, are there other benefits to using coreboot as a core firmware in QEMU? Is there a payload we would ever plausibly use besides OVMF and SeaBIOS? I like the idea of using Coreboot on the UEFI side — if the most actively used TianoCore platform is CorebootPkg instead of OvmfPkg, that makes it a lot easier for people using *real* hardware with Coreboot to use TianoCore. And it helps to dispel the stupid misconception in some quarters that Coreboot *competes* with UEFI and thus cannot possibly be supported because helping something that competes with UEFI would be bad. Is there a payload we would ever plausibly use besides OVMF and SeaBIOS? For my part I want to get to the point where the default firmware shipped with qemu can be OVMF. We have SeaBIOS-as-CSM working, which was one of the biggest barriers. There are a few more things (like NV variable storage, in particular) that I need to fix before I can actually make that suggestion with a straight face though... -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] KVM call agenda for 2013-05-28
On Thu, 2013-05-30 at 13:13 +0200, Laszlo Ersek wrote: Where is CorebootPkg available from? https://github.com/pgeorgi/edk2/tree/coreboot-pkg And it helps to dispel the stupid misconception in some quarters that Coreboot *competes* with UEFI and thus cannot possibly be supported because helping something that competes with UEFI would be bad. I'm not sure who do you mean by some quarters, but for some distributions Coreboot would be yet another component (package) to support, for no obvious benefit. (Gerd said it better than I possibly could: http://thread.gmane.org/gmane.comp.bios.coreboot.seabios/5685/focus=5705.) Yeah, but if we're shoving a lot of hardware-specific ACPI table generation into the guest's firmware, instead of just doing it on the qemu side where a number of us seem to think it belongs, then there *is* a benefit to using Coreboot. When stuff changes on the qemu side and we have to update the table generation to match, you end up having to update just the Coreboot package, and *not* having to patch both SeaBIOS and OVMF. The extra package in the distro really isn't painful to handle, and I suspect it would end up *reducing* the amount of work that we have to do to update. You update *just* Coreboot, not *both* of SeaBIOS and OVMF. If you implement a private UEFI FAT driver from scratch, or port a free software FAT implementation (eg. the r/o one in grub or the r/w one in mtools), you could still run into legal problems, I've been told. That has been said, and it's been said for the FAT implementation in the kernel too. If a distribution is happy to ship the kernel without ripping out its FAT support, then I see no reason why that distribution wouldn't also be happy to ship a version of OVMF with a clean implementation of FAT support. It doesn't make sense to be happy with one but not the other. We need at least one out-of-tree edk2 patch for now (from you) but apparently that's no problem. That'll get merged soon. We are working on the corresponding spec update... As far as I understand: - Jordan is nearing completion of flash support on KVM, - he also has WIP NvVar storage on top of qemu flash. http://thread.gmane.org/gmane.comp.emulators.qemu/213690 http://thread.gmane.org/gmane.comp.bios.tianocore.devel/2781/focus=2798 (Please coordinate I guess :)) Ooh, shiny. Yeah, when I get back to actually working on it rather than just heckling, I'll make sure I do :) -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [PATCH RFC 00/13] qemu: generate acpi tables for the guest
On Tue, 2013-05-14 at 10:38 +0100, Peter Maydell wrote: It depends. For ARM we insist that the user provides the device tree that corresponds to the kernel they're going to run, and then we just tweak it a bit. Um... device trees describe hardware, and should not be at all kernel-specific. Did you mean to say the device tree that corresponds to the machine they're going to emulate? And I suppose you do have a kernel that corresponds to the machine it's going to run on, so what you say isn't *entirely* bogus. But it's just written in a way which makes it scary :) -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] unaligned option rom
On Tue, 2013-04-30 at 09:26 +0300, Michael S. Tsirkin wrote: Each time I boot seabios built with debug enabled, I see this on serial console: Scan for VGA option rom WARNING! Found unaligned PCI rom (vd=1013:00b8) Is this a bogus warning? No. It's warning that your VGA ROM is broken and violates the spec by having the PCIR table at a location which isn't a multiple of 4. SeaBIOS loads it anyway, but we added the relatively harmless warning after discovering that TianoCore refuses to do so. Perhaps you are using a version of the VGA BIOS which still doesn't have this fix applied: http://lists.gnu.org/archive/html/qemu-devel/2013-01/msg03650.html -- David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH 2/2] accept MADT over fw_cfg
On Thu, 2013-03-21 at 13:49 +0100, Gerd Hoffmann wrote: How about we don't bother to determine this at runtime at all? Because it will be a PITA for testers + developers to figure the correct .config switches of the day during the transition phase? Why is it a PITA? Are you developing QEMU? Just use the makefile from roms/config.seabios Are you using QEMU binary? Just use the defaults. SeaBIOS binaries are running on a wide range of qemu versions today. Changing that is a big deal. People are not used to it, and even when writing it to the README people will stumble over it. It also is pretty inconvenient in a number of cases. For starters the usual way to package seabios and qemu in distros is to have separate packages ... I agree that for the foreseeable future, we should be able to build SeaBIOS such that it can cope with old versions of Qemu that *don't* provide ACPI tables. And of course we should make it cope with new versions of Qemu that *do*. But I'm not sure I see any point in doing it table-by-table. Surely it can be all or nothing? -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH 2/2] accept MADT over fw_cfg
On Thu, 2013-03-21 at 13:56 +0100, Laszlo Ersek wrote: - for an earlier qemu, the option must be set, - for a later qemu the option must be clear (no -acpitable switch must be specified on the qemu cmldine || one -acpitable switch must load a MADT) Hm, that sounds like it won't be possible to build one version of SeaBIOS that works for *both* old and new qemu. That doesn't seem like a great idea. I'd prefer something like: - If Qemu provides the 'core' ACPI tables (i.e. not just SSDT) then SeaBIOS uses them. - Otherwise, it makes its own. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH 2/2] accept MADT over fw_cfg
On Thu, 2013-03-21 at 15:12 +0200, Michael S. Tsirkin wrote: On Thu, Mar 21, 2013 at 01:04:35PM +, David Woodhouse wrote: On Thu, 2013-03-21 at 13:56 +0100, Laszlo Ersek wrote: - for an earlier qemu, the option must be set, - for a later qemu the option must be clear (no -acpitable switch must be specified on the qemu cmldine || one -acpitable switch must load a MADT) Hm, that sounds like it won't be possible to build one version of SeaBIOS that works for *both* old and new qemu. That doesn't seem like a great idea. I'd prefer something like: - If Qemu provides the 'core' ACPI tables (i.e. not just SSDT) then SeaBIOS uses them. - Otherwise, it makes its own. -- dwmw2 It might simplify life for someone bisecting qemu as you don't need to rebuild seabios after each bisect but is this really a common workflow? Not bisection, but perhaps switching between an old and a new version of qemu. Anyway, I am not against such runtime flags. If we add to this an option to build a minimal BIOS that only works with the new QEMU, do we have a deal? Yeah, definitely. The code for SeaBIOS to build its own should certainly be optional. It should be possible to build a minimal SeaBIOS which *can't*. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH 2/2] accept MADT over fw_cfg
On Wed, 2013-03-20 at 20:22 -0400, Kevin O'Connor wrote: On Wed, Mar 20, 2013 at 10:53:05PM +0100, Laszlo Ersek wrote: Signed-off-by: Laszlo Ersek ler...@redhat.com I think we need to figure out what the final fw_cfg interface for ACPI, SMBIOS, mptable, and PIR will be. Once we have consensus, we can implement this on the OVMF side too. Is anyone (Laszlo?) looking at that, or should I? For SMBIOS, I don't think we should use the existing fw_cfg mechanism. It's too complicated for what is needed. Instead, one fw_cfg file entry with the whole smbios table should suffice. Agreed. I'd already looked at doing this in OVMF, and run away screaming. For mptable and PIR, there is no current mechanism, so we can add new fw_cfg files for them. However, for all of these SeaBIOS needs to be able to determine when it needs to create the table and when no table should be published at all. I'd make it all-or-nothing. Except for a few historical qemu commits if you're bisecting, why would qemu ever provide a *partial* set of tables? -- Sent with MeeGo's ActiveSync support. David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-stable] problems with freeBSD
On Fri, 2013-03-08 at 08:47 +0100, Paolo Bonzini wrote: Wasn't that a fix for the SeaBIOS VGA BIOS? The one in qemu.git is still built from the Bochs VGA BIOS. No, it was for Bochs VGA BIOS. http://lists.gnu.org/archive/html/qemu-devel/2013-01/msg03650.html -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] Moving BIOS tables from SeaBIOS to QEMU
On Mon, 2013-02-25 at 15:46 +0100, Gerd Hoffmann wrote: 2. Having (many!) hypervisor-specific special cases in SeaBIOS seems wildly schizophrenic without bringing any significant benefits, compared to factoring all of that out into a codebase which *already does many of the needed things*. It's a tradeoff. On one hand letting coreboot handle hardware initialialization would reduce the amout of code in seabios we have to maintain. On the other hand adding coreboot as middle man between qemu and seabios would add some complexity to the whole mix. But if we do it *without* coreboot, then we get to reimplement the whole seabios-qemu-seabios ping-pong, as Laszlo describes it, in Tianocore as *well* as SeaBIOS. I'm not sure we really want to duplicate that code, which looks like it will have tricky interactions between host and guest. When viewed that way, it's not clear that doing it in coreboot is really adding complexity; in that respect it *simplifies* things a bit. Using coreboot everywhere sounds good to me. Especially because on the Tianocore side we can push for Patrick's CorebootPkg to be the *primary* platform; we could even consider deprecating the qemu-specific OvmfPkg completely. And *that* in turn ensures that what everyone's working on is something that ought to be suitable for real hardware, rather than just qemu. It would also help some people on the UEFI side to get over their bizarre misconception that coreboot is antithetical to UEFI :) I'd be quite happy to get to the point where the default firmware for Qemu is Coreboot + Tianocore + SeaBIOS (as CSM). -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] Moving BIOS tables from SeaBIOS to QEMU
On Mon, 2013-02-25 at 15:46 +0100, Gerd Hoffmann wrote: I'm not convinced using coreboot is a clear win, especially with EFI coming. Can coreboot run tianocore as payload? It's being worked on. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH 1/4] Fix return type of le64_to_cpu() and be64_to_cpu()
On Sat, 2013-02-23 at 10:00 -0500, Kevin O'Connor wrote: Patch 2 and 3 look okay to me - if there are no further comments I'll push them. I think we're fairly happy with them. Laszlo put together the OVMF side (creating ACPI 2.0 tables instead of 1.0 and filling in the RESET_REG) and I've tested that it's doing the right thing. We should probably do something similar on the SeaBIOS side when it's creating its own tables, and feed same to acpi_set_reset_reg(). I'm not sure about patch 4. I do think we want to try the standard ways before poking at unstandard ports. Yeah, maybe. Perhaps we could enable that only if PCI devices 8086/7113 or 8086/2918 were found in pci_probe_devices() ? Then again, if we *do* find those devices then our ACPI tables should have given us 0xcf9 as a RESET_REG. So perhaps patch #4 is superfluous. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH 1/4] Fix return type of le64_to_cpu() and be64_to_cpu()
On Sat, 2013-02-23 at 11:38 -0500, Kevin O'Connor wrote: On Sat, Feb 23, 2013 at 04:28:06PM +, David Woodhouse wrote: On Sat, 2013-02-23 at 10:00 -0500, Kevin O'Connor wrote: Patch 2 and 3 look okay to me - if there are no further comments I'll push them. I think we're fairly happy with them. Laszlo put together the OVMF side (creating ACPI 2.0 tables instead of 1.0 and filling in the RESET_REG) and I've tested that it's doing the right thing. We should probably do something similar on the SeaBIOS side when it's creating its own tables, and feed same to acpi_set_reset_reg(). IMO, we need to move the ACPI table creation (and PIR/MPTABLE/SMBIOS) to QEMU and just have QEMU pass the tables to SeaBIOS for it to copy into memory like it does on CSM, coreboot, and Xen. I believe it's on Laszlo's TODO list. If you want to call acpi_set_reset_reg() from pciinit.c (like the way pmtimer_setup() is called) or something similar, that's fine with me. If we actually *created* ACPI 2.0 tables with a RESET_REG I'd have done so already. As it is, I'm happy to wait until we do so, or until we do, or until we get the tables from Qemu. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [edk2] [PATCH 0/8] OvmfPkg: reset-related changes
On Fri, 2013-02-22 at 04:23 +0100, Laszlo Ersek wrote:
Patches 1 to 6 upgrade the FADT to ACPI 2.0 and present the PIIX3
Reset Control Register in it.
Hm. I can still trigger the soft reset loop in SeaBIOS, but perhaps not
in a circumstance that we care about...
I've fixed Qemu to do the proper hard reset on 0xcf9 but not any other
reset method. I'm running with KVM on an old machine without
unrestricted guest support.
I've fixed SeaBIOS to look for the ACPI RESET_REG and use it, and that
part *works*... but SeaBIOS only *sees* the ACPI table when we boot a
legacy OS. If I boot an EFI OS and then do a soft reset via the keyboard
controller ('echo -en \\xfe | dd of=/dev/port bs=1 seek=$((0x64))'),
SeaBIOS hasn't seen the ACPI tables, still uses the keyboard controller
as its first choice of reset method, and still does its reset loop as
before.
Do we actually *care* about that? Do we expect any EFI OS to be doing a
keyboard reset instead of using ACPI or the runtime services Reset call?
If we've booted a legacy OS, it works fine. SeaBIOS gets invoked at
0x0 after a soft reset, uses the ACPI RESET_REG that it was given by
OVMF, and does a proper reset...
# echo -en \\xfe | dd of=/dev/port bs=1 seek=$((0x64))
Changing serial settings was 0/0 now 3/0
In resume (status=0)
In 32bit resume
Attempting a hard reboot
Using ACPI reset reg
rcr_write 6 (debug from qemu)
SecCoreStartupWithStack(0xFFFE6000, 0x8)
File-Type: 0xB
...
--
dwmw2
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] [PATCH 2/4] Rename find_pmtimer() to find_acpi_features()
From: David Woodhouse david.woodho...@intel.com
I'm about to make it do more than just the pmtimer...
Signed-off-by: David Woodhouse david.woodho...@intel.com
---
src/acpi.c | 10 --
src/acpi.h | 2 +-
src/coreboot.c | 4 ++--
src/csm.c | 4 ++--
src/xen.c | 4 ++--
5 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/src/acpi.c b/src/acpi.c
index c7177c3..36bd39a 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -925,15 +925,13 @@ find_resume_vector(void)
}
void
-find_pmtimer(void)
+find_acpi_features(void)
{
struct fadt_descriptor_rev1 *fadt = find_fadt();
if (!fadt)
return;
-u32 pm_tmr = fadt-pm_tmr_blk;
+u32 pm_tmr = le32_to_cpu(fadt-pm_tmr_blk);
dprintf(4, pm_tmr_blk=%x\n, pm_tmr);
-if (!pm_tmr)
-return;
-
-pmtimer_setup(pm_tmr, 3579);
+if (pm_tmr)
+pmtimer_setup(pm_tmr, 3579);
}
diff --git a/src/acpi.h b/src/acpi.h
index e52470e..b23717a 100644
--- a/src/acpi.h
+++ b/src/acpi.h
@@ -5,7 +5,7 @@
void acpi_setup(void);
u32 find_resume_vector(void);
-void find_pmtimer(void);
+void find_acpi_features(void);
#define RSDP_SIGNATURE 0x2052545020445352LL // RSD PTR
diff --git a/src/coreboot.c b/src/coreboot.c
index f0484e1..c9ad2a8 100644
--- a/src/coreboot.c
+++ b/src/coreboot.c
@@ -12,7 +12,7 @@
#include boot.h // boot_add_cbfs
#include disk.h // MAXDESCSIZE
#include config.h // CONFIG_*
-#include acpi.h // find_pmtimer
+#include acpi.h // find_acpi_features
#include pci.h // pci_probe_devices
@@ -214,7 +214,7 @@ coreboot_platform_setup(void)
scan_tables(m-start, m-size);
}
-find_pmtimer();
+find_acpi_features();
}
diff --git a/src/csm.c b/src/csm.c
index 68f8830..4336e16 100644
--- a/src/csm.c
+++ b/src/csm.c
@@ -146,12 +146,12 @@ handle_csm_0002(struct bregs *regs)
dprintf(3, CSM PIRQ table at %p\n, PirAddr);
}
-// For find_resume_vector()... and find_pmtimer()
+// For find_resume_vector()... and find_acpi_features()
if (csm_rsdp.signature == RSDP_SIGNATURE) {
RsdpAddr = csm_rsdp;
dprintf(3, CSM ACPI RSDP at %p\n, RsdpAddr);
-find_pmtimer();
+find_acpi_features();
}
// SMBIOS table needs to be copied into the f-seg
diff --git a/src/xen.c b/src/xen.c
index db542c3..5dfee9e 100644
--- a/src/xen.c
+++ b/src/xen.c
@@ -10,7 +10,7 @@
#include memmap.h // add_e820
#include types.h // ASM32FLAT
#include util.h // copy_acpi_rsdp
-#include acpi.h // find_pmtimer
+#include acpi.h // find_acpi_features
#define INFO_PHYSICAL_ADDRESS 0x1000
@@ -125,7 +125,7 @@ void xen_biostable_setup(void)
for (i=0; iinfo-tables_nr; i++)
copy_table(tables[i]);
-find_pmtimer();
+find_acpi_features();
}
void xen_ramsize_preinit(void)
--
1.8.1.2
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] [PATCH 1/4] Fix return type of le64_to_cpu() and be64_to_cpu()
From: David Woodhouse david.woodho...@intel.com
Signed-off-by: David Woodhouse david.woodho...@intel.com
---
src/byteorder.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/byteorder.h b/src/byteorder.h
index 5a8a64a..7362aeb 100644
--- a/src/byteorder.h
+++ b/src/byteorder.h
@@ -43,7 +43,7 @@ static inline u16 le16_to_cpu(u16 x) {
static inline u32 le32_to_cpu(u32 x) {
return x;
}
-static inline u32 le64_to_cpu(u64 x) {
+static inline u64 le64_to_cpu(u64 x) {
return x;
}
@@ -62,7 +62,7 @@ static inline u16 be16_to_cpu(u16 x) {
static inline u32 be32_to_cpu(u32 x) {
return swab32(x);
}
-static inline u32 be64_to_cpu(u64 x) {
+static inline u64 be64_to_cpu(u64 x) {
return swab64(x);
}
--
1.8.1.2
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] [PATCH 3/4] Add acpi_reboot() reset method using RESET_REG
From: David Woodhouse david.woodho...@intel.com
Signed-off-by: David Woodhouse david.woodho...@intel.com
---
src/acpi.c | 58 +++---
src/acpi.h | 14 ++
src/resume.c | 3 +++
3 files changed, 64 insertions(+), 11 deletions(-)
diff --git a/src/acpi.c b/src/acpi.c
index 36bd39a..195dc88 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -151,17 +151,6 @@ struct madt_local_nmi {
/*
- * ACPI 2.0 Generic Address Space definition.
- */
-struct acpi_20_generic_address {
-u8 address_space_id;
-u8 register_bit_width;
-u8 register_bit_offset;
-u8 reserved;
-u64 address;
-} PACKED;
-
-/*
* HPET Description Table
*/
struct acpi_20_hpet {
@@ -934,4 +923,51 @@ find_acpi_features(void)
dprintf(4, pm_tmr_blk=%x\n, pm_tmr);
if (pm_tmr)
pmtimer_setup(pm_tmr, 3579);
+
+// Theoretically we should check the 'reset_reg_sup' flag, but Windows
+// doesn't and thus nobody seems to *set* it. If the table is large enough
+// to include it, let the sanity checks in acpi_set_reset_reg() suffice.
+if (fadt-length = 129) {
+void *p = fadt;
+acpi_set_reset_reg(p + 116, *(u8 *)(p + 128));
+}
+}
+
+static struct acpi_20_generic_address acpi_reset_reg;
+static u8 acpi_reset_val;
+
+void
+acpi_reboot(void)
+{
+// Check it passed the sanity checks in acpi_set_reset_reg() and was set
+if (acpi_reset_reg.register_bit_width != 8)
+return;
+
+u64 addr = le64_to_cpu(acpi_reset_reg.address);
+
+dprintf(1, ACPI hard reset %d:%llx (%x)\n,
+acpi_reset_reg.address_space_id, addr, acpi_reset_val);
+
+switch (acpi_reset_reg.address_space_id) {
+case 0: // System Memory
+ writeb((void *)(u32)addr, acpi_reset_val);
+break;
+case 1: // System I/O
+outb(acpi_reset_val, addr);
+break;
+case 2: // PCI config space
+pci_config_writeb(acpi_ga_to_bdf(addr), addr 0x, acpi_reset_val);
+break;
+}
+}
+
+void
+acpi_set_reset_reg(struct acpi_20_generic_address *reg, u8 val)
+{
+if (!reg || reg-address_space_id 2 ||
+reg-register_bit_width != 8 || reg-register_bit_offset)
+return;
+
+acpi_reset_reg = *reg;
+acpi_reset_val = val;
}
diff --git a/src/acpi.h b/src/acpi.h
index b23717a..6289953 100644
--- a/src/acpi.h
+++ b/src/acpi.h
@@ -3,9 +3,23 @@
#include types.h // u32
+/*
+ * ACPI 2.0 Generic Address Space definition.
+ */
+struct acpi_20_generic_address {
+u8 address_space_id;
+u8 register_bit_width;
+u8 register_bit_offset;
+u8 reserved;
+u64 address;
+} PACKED;
+#define acpi_ga_to_bdf(addr) pci_to_bdf(0, (addr 32) 0x, (addr 16)
0x)
+
void acpi_setup(void);
u32 find_resume_vector(void);
void find_acpi_features(void);
+void acpi_set_reset_reg(struct acpi_20_generic_address *reg, u8 val);
+void acpi_reboot(void);
#define RSDP_SIGNATURE 0x2052545020445352LL // RSD PTR
diff --git a/src/resume.c b/src/resume.c
index b30d62e..784abac 100644
--- a/src/resume.c
+++ b/src/resume.c
@@ -132,6 +132,9 @@ tryReboot(void)
// Setup for reset on qemu.
qemu_prep_reset();
+// Reboot using ACPI RESET_REG
+acpi_reboot();
+
// Try keyboard controller reboot.
i8042_reboot();
--
1.8.1.2
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] [PATCH 4/4] Try pci_reboot() before i8042_reboot()
From: David Woodhouse david.woodho...@intel.com The so-called PCI reboot at 0xCF9 is supposed to be a hard reset, while the keyboard controller is only a soft reset. So try pci_reboot() first. Signed-off-by: David Woodhouse david.woodho...@intel.com --- src/resume.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/resume.c b/src/resume.c index 784abac..97e20b6 100644 --- a/src/resume.c +++ b/src/resume.c @@ -135,12 +135,12 @@ tryReboot(void) // Reboot using ACPI RESET_REG acpi_reboot(); -// Try keyboard controller reboot. -i8042_reboot(); - // Try PCI 0xcf9 reboot pci_reboot(); +// Try keyboard controller reboot. +i8042_reboot(); + // Try triple fault asm volatile(int3); -- 1.8.1.2 ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] Add acpi_reboot() function for invoking ACPI reset
On Tue, 2013-02-19 at 20:09 -0500, Kevin O'Connor wrote: Looks okay to me. I'd like to see the follow up patches that make use of it before committing though. Sent. With Qemu fixed to actually *do* a hard reset, and OVMF fixed to pass in ACPI 2.0 tables with a RESET_REG filled in (thanks, Laszlo), this fixes the endless loop of soft resets. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [RFC PATCH] Distinguish between reset types
On Wed, 2013-02-20 at 16:34 +0100, Paolo Bonzini wrote: Il 20/02/2013 16:18, Laszlo Ersek ha scritto: I'm beginning to wish I'd just ignored the fact that we need a properly working soft reset to get back from 286 protected mode to real mode, and wired up the damn PAM reset unconditionally. I'm not convinced that the protected-real mode transition will work for anyone anyway. I believe currently we must be somewhere between soft reset hard reset. I estimate getting closer to a well-emulated hard reset is more important than getting closer to a soft one. If you were to extend the i440FX reset handler so that it unconditionally resets the PAMs, I'd give my Rb. (Take it for what it's worth :)) It would actually make sense. The right way to do soft reset has nothing to do with qemu_system_reset_request(). I've posted that version of the patch, with a suitable comment. Thanks. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] Move malloc's ZoneFSeg and ZoneLow setup to malloc_init.
On Tue, 2013-02-19 at 01:38 -0500, Kevin O'Connor wrote: This reduces some duplicate code between malloc_preinit() and csm_malloc_preinit(). Signed-off-by: Kevin O'Connor ke...@koconnor.net Nice. I was planning to reduce that duplication, and this seems ideal. Thanks. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] Report on f-segment UMB ram also.
On Mon, 2013-02-18 at 10:34 -0500, Kevin O'Connor wrote: Some old DOS programs can also use f-segment space as Upper Memory Blocks (UMB), so also report on what space is available in debug messages. Should we mark it as available in E820 too? -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM
On Mon, 2013-02-18 at 23:08 +, David Woodhouse wrote:
Laszlo has hooked up the RCR on the PIIX3 already, so something like
this ought to make it reset the PAM setup *only* if reset via that...
+static void i440fx_reset(DeviceState *ds)
+{
+PCIDevice *dev = DO_UPCAST(PCIDevice, qdev, ds);
+PCII440FXState *d = DO_UPCAST(PCII440FXState, dev, dev);
+uint8_t *pci_conf = d-dev.config;
+
+if (!d-piix3-rcr_hard_reset)
+return;
+
... except that bit (referring to PIIX3 state directly from
i440fx_reset(), having stashed a pointer to it) is horrible.
I've posted a 'cleaner' but much larger and more intrusive patch which
shows how we could introduce a 'reset type' as a proper concept, which
may well be useful for other platforms and situations too.
I'm not too bothered which way we go, but it would be very good to fix
the PAM reset in qemu, because it's a genuine fix and it's *extremely*
convenient to work around the KVM CS segment base bug.
--
dwmw2
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Xen-devel] [PATCHv2 0/6] Improved multi-platform support
On Tue, 2013-02-19 at 10:20 +, Ian Campbell wrote: I expect there will be some rough edges like the NV variable thing -- I have a feeling these will have a lot in common with qemu/KVM, since they would tend to interact with the platform (provided by qemu-dm under Xen) rather than processor or memory virt etc. Well, it's mostly just storing text strings describing which device/path to boot from. So hopefully if the flash storage itself is OK for Xen, the rest should just work. For the most part the I reckon the Xen specific things will be resync with a recent upstream and fixup the Xen build system integration, stuff like that. Current upstream OVMF works, AFAICT. Although as I said, I haven't even tried booting an OS. One thing I obviously *also* haven't tested, therefore, is *rebooting*., We've discovered a KVM bug on older CPUs without 'unrestricted guest' mode, where after a reset it actually runs code from 0x0 and not 0xfff0. With the former being SeaBIOS-CSM, and the latter being the OVMF startup code that we *should* be running, that's kind of a problem. If you have a similar bug, please go and fix it so I never have to know :) We'd like to see PV I/O drivers at some point but that's a separate project in its own right I think. If you have an option ROM for them, that might not be a high priority. I think OVMF can thunk into INT 13h for disk access. And on the subject of the NV storage... does -pflash work on qemu-xen, as long as we're not actually running *code* from the device and we're only using it for data storage? I'm not sure, I've CC'd Anthony Stefano. Assuming the Xen machine type in Qemu wires it up then I don't know why it shouldn't work and if we don't wire it up I don't see why we couldn't. Qemu does but not in KVM mode, because it can't *execute* from a region and also catch writes to properly emulate flash. But for a flash device that we *don't* execute from, because it's only used for NV storage and not the firmware itself, that restriction isn't important. So I'll need to fix that. Hvmloader might need to be tweaked to setup the e820 correctly and perhaps the some table or other would need to refer to it also? Right. Haven't quite worked out how it would be 'found' by the firmware yet. I was going to suggest making the address available via fw_cfg... but you don't implement that. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM
On Mon, 2013-02-18 at 17:37 -0500, Kevin O'Connor wrote: The ACPI v2 spec describes a hard reset register. SeaBIOS could extract it from the FADT and then use it. Of course, we'd probably want to update the QEMU ACPI tables to implement ACPI v2 then. This sounded great until I actually came to implement it. The PIIX reset at 0xcf9 requires *two* writes; one to set the reset type and then a second write with bit 2 set to actually do the reset. The ACPI RESET_REG definition only allows for *one* value to be written. Is that because the PIIX will actually do a hard reset when you write 0x06 to it *anyway*, despite theoretically saying that you should write 0x02 first? Or is the ACPI definition of RESET_REG simply incapable of being used on the PIIX? -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] [PATCH] Add acpi_reboot() function for invoking ACPI reset
Signed-off-by: David Woodhouse david.woodho...@intel.com
---
Nothing actually sets it yet. We'll do that from the ACPI table parser
for CSM and Xen, and we can put it in our own tables for the native
case.
diff --git a/src/acpi.c b/src/acpi.c
index f7a2e55..97ade3f 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -150,17 +150,6 @@ struct madt_local_nmi {
/*
- * ACPI 2.0 Generic Address Space definition.
- */
-struct acpi_20_generic_address {
-u8 address_space_id;
-u8 register_bit_width;
-u8 register_bit_offset;
-u8 reserved;
-u64 address;
-} PACKED;
-
-/*
* HPET Description Table
*/
struct acpi_20_hpet {
@@ -936,3 +925,38 @@ find_pmtimer(void)
pmtimer_setup(pm_tmr, 3579);
}
+
+static struct acpi_20_generic_address acpi_reset_reg;
+static u8 acpi_reset_val;
+
+void
+acpi_reboot(void)
+{
+// Must be a single byte;
+if (acpi_reset_reg.register_bit_width != 8)
+return;
+
+switch (acpi_reset_reg.address_space_id) {
+case 0: // System Memory
+writeb((void *)(u32)acpi_reset_reg.address, acpi_reset_val);
+break;
+case 1: // System I/O
+outb(acpi_reset_val, acpi_reset_reg.address);
+break;
+case 2: // PCI config space
+pci_config_writeb(acpi_ga_to_bdf(acpi_reset_reg.address),
+ acpi_reset_reg.address 0x,
+ acpi_reset_val);
+break;
+}
+}
+
+void acpi_set_reset_reg(struct acpi_20_generic_address *reg, u8 val)
+{
+if (!reg || reg-address_space_id 2 ||
+reg-register_bit_width != 8 || reg-register_bit_offset)
+return;
+
+acpi_reset_reg = *reg;
+acpi_reset_val = val;
+}
diff --git a/src/acpi.h b/src/acpi.h
index e52470e..3f85814 100644
--- a/src/acpi.h
+++ b/src/acpi.h
@@ -3,9 +3,23 @@
#include types.h // u32
+/*
+ * ACPI 2.0 Generic Address Space definition.
+ */
+struct acpi_20_generic_address {
+u8 address_space_id;
+u8 register_bit_width;
+u8 register_bit_offset;
+u8 reserved;
+u64 address;
+} PACKED;
+#define acpi_ga_to_bdf(addr) pci_to_bdf(0, (addr 32) 0x, (addr 16)
0x)
+
void acpi_setup(void);
u32 find_resume_vector(void);
void find_pmtimer(void);
+void acpi_set_reset_reg(struct acpi_20_generic_address *reg, u8 val);
+void acpi_reboot(void);
#define RSDP_SIGNATURE 0x2052545020445352LL // RSD PTR
diff --git a/src/resume.c b/src/resume.c
index adc3594..6b9ad9a 100644
--- a/src/resume.c
+++ b/src/resume.c
@@ -132,6 +132,9 @@ tryReboot(void)
// Setup for reset on qemu.
qemu_prep_reset();
+// Reboot using ACPI RESET_REG
+acpi_reboot();
+
// Try keyboard controller reboot.
i8042_reboot();
--
dwmw2
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM
On Tue, 2013-02-19 at 20:13 +0200, Gleb Natapov wrote: I take it you mean copy 0xfffe to 0xe? That would not be fun. SeaBIOS would need to detect that it's in the state (it's definitely not correct to do that on real-hardware or on working kvm instances), then setup a trampoline somewhere outside of 0xe-0xf to do the memcpy, jump to that trampoline, copy the memory, restore segment registers, and then jump to 0xfff0. That's a lot of kvm specific code to add to seabios as a workaround and it seems fragile anyway. Isn't this exactly what qemu_prep_reset() is doing now? No. It doesn't do the trampoline thing because it doesn't *have* to; it's copying an identical copy of the code back over itself. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM
On Tue, 2013-02-19 at 20:41 +0200, Gleb Natapov wrote: Ah, yes of course. So does CSM takes the whole 0xe-0xf segment or it leaves OVMF code there somewhere. CSM reset code can jump into OVMF code in 0xe-0xf range and let it do the copy. There is no OVMF code there; OVMF doesn't bother to put *anything* into the RAM at 1MiB-δ unless there's a CSM. CSM code isn't supposed to be hardware-specific, but I suppose for the CSM running under KVM case we could *potentially* have a hack at the reset vector so that when we do find ourselves there under a buggy qemu/KVM implementation, it could set up a trampoline, reset the PAM registers manually (so that the KVM CS base address bug doesn't actually *hurt* us), then try again? I'd rather implement the 0xcf9 reset properly in qemu though, and make SeaBIOS use that (which it can do *sanely* as a CSM if it's in the ACPI tables). -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM
On Tue, 2013-02-19 at 21:01 +0200, Gleb Natapov wrote: On Tue, Feb 19, 2013 at 06:48:41PM +, David Woodhouse wrote: On Tue, 2013-02-19 at 20:41 +0200, Gleb Natapov wrote: Ah, yes of course. So does CSM takes the whole 0xe-0xf segment or it leaves OVMF code there somewhere. CSM reset code can jump into OVMF code in 0xe-0xf range and let it do the copy. There is no OVMF code there; OVMF doesn't bother to put *anything* into the RAM at 1MiB-δ unless there's a CSM. It runs from ROM and do not shadow itself? It has no need to shadow itself. It loads the SeaBIOS CSM into the range under 1MiB, if it wants to support legacy BIOS. Other than that, it never cares about 16-bit code at all. I'd rather implement the 0xcf9 reset properly in qemu though, and make SeaBIOS use that (which it can do *sanely* as a CSM if it's in the ACPI tables). I didn't follow that other discussion about hard/soft reset. How proper 0xcf9 reset will fix the problem? What will it do that system_reset does not? A full *hard* reset (0xcf9) will reset the PAM configuration, and thus the BIOS from 4GiB-δ *would* be shadowed into 1MiB-δ, by hardware. But qemu doesn't *implement* a full hard reset; it doesn't reset the PAM registers. And making it do so the naïve way, by just hooking up a simple device reset function to do so, would be wrong. Because it *shouldn't* happen on a soft reset, such as a triple-fault or a reset triggered by the keyboard controller. Since a soft reset was the only way to get back from 80286 protected mode to 8086 mode, some software may actually *use* it and expect it to behave correctly. Hence the discussion about reset handling. We'd need to fix SeaBIOS to use the 0xcf9 reset too; currently it'll sit in an endless loop of keyboard-induced *soft* resets anyway, because it tries that before 0xcf9. And in fact it probably shouldn't use the hard-coded 0xcf9 reset; it should use the one indicated by the ACPI RESET_REG field (which *is* 0xcf9... or should be). -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM
On Tue, 2013-02-19 at 21:49 +0100, Paolo Bonzini wrote: And in fact it probably shouldn't use the hard-coded 0xcf9 reset; it should use the one indicated by the ACPI RESET_REG field (which *is* 0xcf9... or should be). We should implement this: http://mjg59.dreamwidth.org/3561.html Matthew fails to distinguish between a hard reset and a soft reset. From the CSM if we do find ourselves running at 0x0 (which should never happen except under buggy KVM emulation anyway), we really do need to be using the 0xcf9 reset (or the ACPI reset, which is going to point to the same thing in general), and *not* the keyboard reset. And, of course, we need it to work correctly and reset the PAM configuration (qv). However, a single bash on the 0xcf9 register ought to suffice so the ACPI/kbd/ACPI/kbd loop that Matthew describes is probably acceptable. As long as it does the ACPI one *first*. ( It's also interesting that, as Laszlo observes, machines tend to set the RESET_REG in the FADT *without* setting the enabled bit in the FADT flags. Does Windows use it anyway? And is there are reason for *not* setting the enabled bit, or is it just that all PC BIOSes are written by crack-smoking hobos that they drag in off the street, and this is just an artefact of the rule anything they *can* get wrong and still boot Windows, they *will* get wrong? ) -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM
On Sat, 2013-02-16 at 02:37 +0100, Laszlo Ersek wrote: I give up. Thanks for the help sorry about spamming three lists. I've managed to reproduce this on a clean F18 system. This is the stock qemu 1.2.2-6.fc18 on kernel 3.7.6-201.fc18.x86_64 with a newly-installed Fedora 18 VM in the guest. qemu-system-x86_64 -enable-kvm -cdrom F18boot.iso -serial mon:stdio -bios OVMF.fd On my laptop where I'd been doing most of my testing, even after running 'yum distro-sync qemu\*' to get back to the stock qemu, I still can't reproduce the issue. They are both running the *same* kernel. I'll try reverting a whole bunch of other stuff that ought to be irrelevant to the stock distro packages, and see if/when it breaks... -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM
On Mon, 2013-02-18 at 10:40 +, David Woodhouse wrote:
On Sat, 2013-02-16 at 02:37 +0100, Laszlo Ersek wrote:
I give up. Thanks for the help sorry about spamming three lists.
I've managed to reproduce this on a clean F18 system. This is the stock
qemu 1.2.2-6.fc18 on kernel 3.7.6-201.fc18.x86_64 with a newly-installed
Fedora 18 VM in the guest.
qemu-system-x86_64 -enable-kvm -cdrom F18boot.iso -serial mon:stdio -bios
OVMF.fd
On my laptop where I'd been doing most of my testing, even after running
'yum distro-sync qemu\*' to get back to the stock qemu, I still can't
reproduce the issue. They are both running the *same* kernel.
I'll try reverting a whole bunch of other stuff that ought to be
irrelevant to the stock distro packages, and see if/when it breaks...
I cannot make these two machines behave consistently. I have absolutely
no clue what is going on here.
At reset, the PAM regions are all set to '1' (read only). So the CSM
should reside in RAM at 0x0 but THAT SHOULDN'T MATTER. After a reset
we should be running from 0xfff0 and there's unconditionally ROM
there, isn't there?
Nevertheless, on my workstation as on yours, we do seem to end up
executing from the CSM in RAM when we reset. But on my laptop, it
executes the *ROM* as it should.
This patch 'fixes' it, and I think it might even be correct in itself,
but I don't think it's a correct fix for the problem we're discussing.
And I certainly want to know what's different on my laptop that makes it
work *without* this patch.
Either there's some weirdness with setting the high CS base address, on
CPU reset. Or perhaps the contents of the memory region at 0xfff0
have *really* been changed along with the sub-1MiB range. Or maybe the
universe just hates us...
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 6c77e49..6dcf1c5 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -171,6 +171,23 @@ static int i440fx_load_old(QEMUFile* f, void *opaque, int
version_id)
return 0;
}
+static void i440fx_reset(void *opaque)
+{
+PCII440FXState *d = opaque;
+uint8_t *pci_conf = d-dev.config;
+
+pci_conf[0x59] = 0x00; // Reset PAM setup
+pci_conf[0x5a] = 0x00;
+pci_conf[0x5b] = 0x00;
+pci_conf[0x5c] = 0x00;
+pci_conf[0x5d] = 0x00;
+pci_conf[0x5e] = 0x00;
+pci_conf[0x5f] = 0x00;
+pci_conf[0x72] = 0x02; // And SMM
+
+i440fx_update_memory_mappings(d);
+}
+
static int i440fx_post_load(void *opaque, int version_id)
{
PCII440FXState *d = opaque;
@@ -217,6 +234,8 @@ static int i440fx_initfn(PCIDevice *dev)
d-dev.config[I440FX_SMRAM] = 0x02;
cpu_smm_register(i440fx_set_smm, d);
+
+qemu_register_reset(i440fx_reset, d);
return 0;
}
--
dwmw2
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM
On Mon, 2013-02-18 at 15:46 +0100, Paolo Bonzini wrote: If you want to submit this patch for upstream QEMU (I agree it is a good idea), please set dc-reset instead in i440fx_class_init. Thanks. I just copied the way that PIIX3 does it... is that something that piix3_class_init() should be doing for *its* reset function too? I'll submit this for upstream, but I consider it a workaround for the real bug that Laszlo has been suffering from. So I'd rather wait until we've solved that properly, or at least until we understand why we get such different results on different CPUs. -- David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM
On Mon, 2013-02-18 at 19:16 +0100, Laszlo Ersek wrote: On 02/18/13 18:45, Gleb Natapov wrote: On Mon, Feb 18, 2013 at 06:12:55PM +0100, Laszlo Ersek wrote: CS =f000 000f f300 ^^^^ |base limitflags selector This is because real mode is emulated as vm86 mode on intel cpus without unrestricted guest flag. Awesome, this supports mys desperate hunch in http://lists.nongnu.org/archive/html/qemu-devel/2013-02/msg02689.html. I hope David can confirm in practice! Yes, my working machines have unrestricted_guest support, and the non-working machines don't. So when we're emulating it in vm86, the extended segment base handling is broken. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM
On Mon, 2013-02-18 at 14:00 -0500, Kevin O'Connor wrote: On Mon, Feb 18, 2013 at 08:31:01PM +0200, Gleb Natapov wrote: Laszlo explained to me that the problem is that after reset we end up in SeaBIOS reset code instead of OVMF one. This is because kvm starts to execute from 0 instead of fff0 after reset and this memory location is modifying during CSM loading. Seabios solves this problem by detecting reset condition and copying pristine image of itself from the end of 4G to the end of 1M. OVMF should do the same, but with CSM it does not get control back after reset since Seabios reset vector is executed instead. Why not put OVMF reset code at reset vector in CSM built SeaBIOS to solve the problem? Why not fix KVM so that it runs at fff0 after reset? The only thing SeaBIOS could do is setup the segment registers and then jump to fff0, which is a bit of work for the same end result. Well, what SeaBIOS already *does* is bash on the keyboard controller to cause a reset. Which *ought* to work too; I have a patch to at least fix *that*, by resetting the PAM setup in the i440. But yes, KVM definitely ought to be running at 0xfff0. This is the *vm86* code that's broken, not the native KVM version. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM
On Mon, 2013-02-18 at 14:11 -0500, Kevin O'Connor wrote: On Mon, Feb 18, 2013 at 07:04:08PM +, David Woodhouse wrote: Well, what SeaBIOS already *does* is bash on the keyboard controller to cause a reset. Which *ought* to work too; I have a patch to at least fix *that*, by resetting the PAM setup in the i440. The thing to be aware of here is that not all resets are equal. There is old code out there that will force a reset to go from 80386 mode to 8086 mode (or was it 286 to 8086?). So, some resets are really resumes (which must not alter memory) and some are real resets. It's a mystery to me which is which, but I know this came up the last time the QEMU reset logic was discussed. Hm, yes. It will have been 286 to 8086, because ISTR there was no *other* way for the CPU to get back from 286 mode. The i440fx data sheet (§3.0) appears to say that the default values are loaded on a *hard* reset, not a soft reset. And a reset invoked by the keyboard controller (as SeaBIOS does) is a *soft* reset. The only way to do a *hard* reset from software that's mentioned in the datasheet is the PMC turbo/reset control register (port 0x93). And that, presumably, is chipset-dependent and not something we can easily use from the reset vector without doing a bunch of hardware probing. I suppose we could set it up in advance, during the *first* initialisation. Just point a 'do_hard_reset()' function pointer at a function of our choice, perhaps with the existing keyboard reset as a default if we don't know of anything better. So we could probably solve the software side, in the guest... but qemu doesn't seem to distinguish between a hard reset and a soft reset, so there's no way to make it reset the PAM registers in one case but not the other. Does this reset for 286-8086 mode actually work in qemu at all? Is qemu's reset a hard reset, or a soft reset? I suppose given that the RCR is part of the I440FX, and the behaviour that we want to vary for hard vs. soft reset is also within the I440FX, we *could* contrive to reset the PAM registers *only* when reset via the RCR. But if I propose a patch which does it that way, will someone hunt me down and hurt me? -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] Enable VGA output when setting Cirrus-specific mode
On Mon, 2013-02-18 at 21:27 +0100, Laszlo Ersek wrote: could someone please review this one-liner? Kevin already merged it. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM
On Mon, 2013-02-18 at 17:37 -0500, Kevin O'Connor wrote:
On Mon, Feb 18, 2013 at 09:12:46PM +, David Woodhouse wrote:
The i440fx data sheet (§3.0) appears to say that the default values are
loaded on a *hard* reset, not a soft reset. And a reset invoked by the
keyboard controller (as SeaBIOS does) is a *soft* reset. The only way to
do a *hard* reset from software that's mentioned in the datasheet is the
PMC turbo/reset control register (port 0x93). And that, presumably, is
chipset-dependent and not something we can easily use from the reset
vector without doing a bunch of hardware probing.
The ACPI v2 spec describes a hard reset register. SeaBIOS could
extract it from the FADT and then use it. Of course, we'd probably
want to update the QEMU ACPI tables to implement ACPI v2 then.
Yeah, that makes me somewhat happier about the SeaBIOS side of it being
hardware-specific. That way the code at the reset vector only has to
cope with a single 8-bit write to memory, IO or config space.
Laszlo has hooked up the RCR on the PIIX3 already, so something like
this ought to make it reset the PAM setup *only* if reset via that...
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 6c77e49..f4420bd 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -77,6 +77,7 @@ typedef struct PIIX3State {
/* Reset Control Register contents */
uint8_t rcr;
+uint8_t rcr_hard_reset;
/* IO memory region for Reset Control Register (RCR_IOPORT) */
MemoryRegion rcr_mem;
@@ -84,6 +85,7 @@ typedef struct PIIX3State {
struct PCII440FXState {
PCIDevice dev;
+PIIX3State *piix3;
MemoryRegion *system_memory;
MemoryRegion *pci_address_space;
MemoryRegion *ram_memory;
@@ -171,6 +173,29 @@ static int i440fx_load_old(QEMUFile* f, void *opaque, int
version_id)
return 0;
}
+static void i440fx_reset(DeviceState *ds)
+{
+PCIDevice *dev = DO_UPCAST(PCIDevice, qdev, ds);
+PCII440FXState *d = DO_UPCAST(PCII440FXState, dev, dev);
+uint8_t *pci_conf = d-dev.config;
+
+if (!d-piix3-rcr_hard_reset)
+return;
+
+pci_conf[0x59] = 0x00; // Reset PAM setup
+pci_conf[0x5a] = 0x00;
+pci_conf[0x5b] = 0x00;
+pci_conf[0x5c] = 0x00;
+pci_conf[0x5d] = 0x00;
+pci_conf[0x5e] = 0x00;
+pci_conf[0x5f] = 0x00;
+pci_conf[0x72] = 0x02; // And SMM
+
+i440fx_update_memory_mappings(d);
+
+d-piix3-rcr_hard_reset = 0;
+}
+
static int i440fx_post_load(void *opaque, int version_id)
{
PCII440FXState *d = opaque;
@@ -297,6 +322,7 @@ static PCIBus *i440fx_common_init(const char *device_name,
pci_bus_set_route_irq_fn(b, piix3_route_intx_pin_to_irq);
}
piix3-pic = pic;
+f-piix3 = piix3;
*isa_bus = DO_UPCAST(ISABus, qbus,
qdev_get_child_bus(piix3-dev.qdev, isa.0));
@@ -521,6 +547,8 @@ static void rcr_write(void *opaque, hwaddr addr, uint64_t
val, unsigned len)
PIIX3State *d = opaque;
if (val 4) {
+if (val 2)
+d-rcr_hard_reset = 1;
qemu_system_reset_request();
return;
}
@@ -615,6 +643,7 @@ static void i440fx_class_init(ObjectClass *klass, void
*data)
dc-desc = Host bridge;
dc-no_user = 1;
dc-vmsd = vmstate_i440fx;
+dc-reset = i440fx_reset;
}
static const TypeInfo i440fx_info = {
--
dwmw2
smime.p7s
Description: S/MIME cryptographic signature
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
