6:17 p.m.
As with the recent change to AHCI, if the pvscsi driver needs to jump
into 32bit mode to access a register, then it's better to run the
pvscsi driver entirely in 32bit mode.
Evgeny, can you take a look at this and give it a test? Also, what
command line do you use for testing?
I tried testing locally by adding:
-device pvscsi,id=pvscsi0 -device scsi-disk,bus=pvscsi0.0,drive=drive0 -drive
id=drive0,if=none,file=dos-drivec-new
to my qemu command line (both qemu v1.6 and qemu v1.7), but it doesn't
work for me even before my changes. It hangs in
pvscsi_wait_intr_cmpl().
-Kevin
Kevin O'Connor (3):
pvscsi: Don't store reference to struct pci_device.
pvscsi: Always run entirely in 32bit mode.
pvscsi: Remove use of LOWFLAT and GLOBALFLAT macros.
Makefile | 4 +--
src/block.c | 8 +++--
src/hw/blockcmd.c | 3 +-
src/hw/pvscsi.c | 106 +++++++++++++++++++++++++-----------------------------
4 files changed, 58 insertions(+), 63 deletions(-)
--
1.8.3.1
6:17 p.m.
New subject: [PATCH 1/3] pvscsi: Don't store reference to struct pci_device.
The pci_device reference isn't used by pvscsi, and it's confusing to
keep a long held reference to a short lived object.
Signed-off-by: Kevin O'Connor <kevin(a)koconnor.net>
---
src/hw/pvscsi.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/hw/pvscsi.c b/src/hw/pvscsi.c
index 6911230..00ce377 100644
--- a/src/hw/pvscsi.c
+++ b/src/hw/pvscsi.c
@@ -129,7 +129,6 @@ struct pvscsi_ring_dsc_s {
struct pvscsi_lun_s {
struct drive_s drive;
- struct pci_device *pci;
u32 iobase;
u8 target;
u8 lun;
@@ -291,7 +290,6 @@ pvscsi_add_lun(struct pci_device *pci, u32 iobase,
memset(plun, 0, sizeof(*plun));
plun->drive.type = DTYPE_PVSCSI;
plun->drive.cntl_id = pci->bdf;
- plun->pci = pci;
plun->target = target;
plun->lun = lun;
plun->iobase = iobase;
--
1.8.3.1
6:18 p.m.
New subject: [PATCH 2/3] pvscsi: Always run entirely in 32bit mode.
Instead of jumping into 32bit mode to access the PCI config space, run
the entire driver in 32bit mode.
Signed-off-by: Kevin O'Connor <kevin(a)koconnor.net>
---
Makefile | 4 ++--
src/block.c | 8 ++++++--
src/hw/blockcmd.c | 3 ++-
src/hw/pvscsi.c | 30 +++++++++++++++---------------
4 files changed, 25 insertions(+), 20 deletions(-)
diff --git a/Makefile b/Makefile
index aba8a62..414c90a 100644
--- a/Makefile
+++ b/Makefile
@@ -34,11 +34,11 @@ SRCBOTH=misc.c stacks.c output.c string.c x86.c block.c cdrom.c
mouse.c kbd.c \
hw/usb-hid.c hw/usb-msc.c hw/usb-uas.c \
hw/blockcmd.c hw/floppy.c hw/ata.c hw/ramdisk.c \
hw/virtio-ring.c hw/virtio-pci.c hw/virtio-blk.c hw/virtio-scsi.c \
- hw/lsi-scsi.c hw/esp-scsi.c hw/megasas.c hw/pvscsi.c
+ hw/lsi-scsi.c hw/esp-scsi.c hw/megasas.c
SRC16=$(SRCBOTH) system.c disk.c font.c
SRC32FLAT=$(SRCBOTH) post.c memmap.c malloc.c pmm.c romfile.c optionroms.c \
boot.c bootsplash.c jpeg.c bmp.c \
- hw/ahci.c hw/usb-hub.c \
+ hw/ahci.c hw/pvscsi.c hw/usb-hub.c \
fw/coreboot.c fw/lzmadecode.c fw/csm.c fw/biostables.c \
fw/paravirt.c fw/shadow.c fw/pciinit.c fw/smm.c fw/mtrr.c fw/xen.c \
fw/acpi.c fw/mptable.c fw/pirtable.c fw/smbios.c fw/romfile_loader.c
diff --git a/src/block.c b/src/block.c
index ac2a830..898c279 100644
--- a/src/block.c
+++ b/src/block.c
@@ -313,7 +313,7 @@ __disk_ret_unimplemented(struct bregs *regs, u32 linecode, const char
*fname)
* 16bit calling interface
****************************************************************/
-static int
+int VISIBLE32FLAT
process_scsi_op(struct disk_op_s *op)
{
switch (op->command) {
@@ -386,9 +386,13 @@ process_op(struct disk_op_s *op)
case DTYPE_LSI_SCSI:
case DTYPE_ESP_SCSI:
case DTYPE_MEGASAS:
- case DTYPE_PVSCSI:
ret = process_scsi_op(op);
break;
+ case DTYPE_PVSCSI: ;
+ extern void _cfunc32flat_process_scsi_op(void);
+ ret = call32(_cfunc32flat_process_scsi_op
+ , (u32)MAKE_FLATPTR(GET_SEG(SS), op), DISK_RET_EPARAM);
+ break;
default:
ret = DISK_RET_EPARAM;
break;
diff --git a/src/hw/blockcmd.c b/src/hw/blockcmd.c
index 97c6675..db61cbd 100644
--- a/src/hw/blockcmd.c
+++ b/src/hw/blockcmd.c
@@ -44,7 +44,8 @@ cdb_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize)
case DTYPE_MEGASAS:
return megasas_cmd_data(op, cdbcmd, blocksize);
case DTYPE_PVSCSI:
- return pvscsi_cmd_data(op, cdbcmd, blocksize);
+ if (!MODESEGMENT)
+ return pvscsi_cmd_data(op, cdbcmd, blocksize);
case DTYPE_AHCI_ATAPI:
if (!MODESEGMENT)
return ahci_cmd_data(op, cdbcmd, blocksize);
diff --git a/src/hw/pvscsi.c b/src/hw/pvscsi.c
index 00ce377..8b4b10f 100644
--- a/src/hw/pvscsi.c
+++ b/src/hw/pvscsi.c
@@ -129,41 +129,41 @@ struct pvscsi_ring_dsc_s {
struct pvscsi_lun_s {
struct drive_s drive;
- u32 iobase;
+ void *iobase;
u8 target;
u8 lun;
struct pvscsi_ring_dsc_s *ring_dsc;
};
static void
-pvscsi_write_cmd_desc(u32 iobase, u32 cmd, const void *desc, size_t len)
+pvscsi_write_cmd_desc(void *iobase, u32 cmd, const void *desc, size_t len)
{
const u32 *ptr = desc;
size_t i;
len /= sizeof(*ptr);
- pci_writel(iobase + PVSCSI_REG_OFFSET_COMMAND, cmd);
+ writel(iobase + PVSCSI_REG_OFFSET_COMMAND, cmd);
for (i = 0; i < len; i++)
- pci_writel(iobase + PVSCSI_REG_OFFSET_COMMAND_DATA, ptr[i]);
+ writel(iobase + PVSCSI_REG_OFFSET_COMMAND_DATA, ptr[i]);
}
static void
-pvscsi_kick_rw_io(u32 iobase)
+pvscsi_kick_rw_io(void *iobase)
{
- pci_writel(iobase + PVSCSI_REG_OFFSET_KICK_RW_IO, 0);
+ writel(iobase + PVSCSI_REG_OFFSET_KICK_RW_IO, 0);
}
static void
-pvscsi_wait_intr_cmpl(u32 iobase)
+pvscsi_wait_intr_cmpl(void *iobase)
{
- while (!(pci_readl(iobase + PVSCSI_REG_OFFSET_INTR_STATUS) &
PVSCSI_INTR_CMPL_MASK))
+ while (!(readl(iobase + PVSCSI_REG_OFFSET_INTR_STATUS) & PVSCSI_INTR_CMPL_MASK))
usleep(5);
- pci_writel(iobase + PVSCSI_REG_OFFSET_INTR_STATUS, PVSCSI_INTR_CMPL_MASK);
+ writel(iobase + PVSCSI_REG_OFFSET_INTR_STATUS, PVSCSI_INTR_CMPL_MASK);
}
static void
-pvscsi_init_rings(u32 iobase, struct pvscsi_ring_dsc_s **ring_dsc)
+pvscsi_init_rings(void *iobase, struct pvscsi_ring_dsc_s **ring_dsc)
{
struct PVSCSICmdDescSetupRings cmd = {0,};
@@ -279,7 +279,7 @@ pvscsi_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize)
}
static int
-pvscsi_add_lun(struct pci_device *pci, u32 iobase,
+pvscsi_add_lun(struct pci_device *pci, void *iobase,
struct pvscsi_ring_dsc_s *ring_dsc, u8 target, u8 lun)
{
struct pvscsi_lun_s *plun = malloc_fseg(sizeof(*plun));
@@ -311,7 +311,7 @@ fail:
}
static void
-pvscsi_scan_target(struct pci_device *pci, u32 iobase,
+pvscsi_scan_target(struct pci_device *pci, void *iobase,
struct pvscsi_ring_dsc_s *ring_dsc, u8 target)
{
/* TODO: send REPORT LUNS. For now, only LUN 0 is recognized. */
@@ -324,12 +324,12 @@ init_pvscsi(struct pci_device *pci)
struct pvscsi_ring_dsc_s *ring_dsc = NULL;
int i;
u16 bdf = pci->bdf;
- u32 iobase = pci_config_readl(pci->bdf, PCI_BASE_ADDRESS_0)
- & PCI_BASE_ADDRESS_MEM_MASK;
+ void *iobase = (void*)(pci_config_readl(pci->bdf, PCI_BASE_ADDRESS_0)
+ & PCI_BASE_ADDRESS_MEM_MASK);
pci_config_maskw(bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER);
- dprintf(1, "found pvscsi at %02x:%02x.%x, io @ %x\n",
+ dprintf(1, "found pvscsi at %02x:%02x.%x, io @ %p\n",
pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf),
pci_bdf_to_fn(bdf), iobase);
--
1.8.3.1
6:18 p.m.
New subject: [PATCH 3/3] pvscsi: Remove use of LOWFLAT and GLOBALFLAT macros.
Now that pvscsi runs entirely in 32bit mode, there is no need to use
the memory segment access macros.
This also fixes up an incorrect memcpy and memset call.
Signed-off-by: Kevin O'Connor <kevin(a)koconnor.net>
---
src/hw/pvscsi.c | 74 +++++++++++++++++++++++++--------------------------------
1 file changed, 33 insertions(+), 41 deletions(-)
diff --git a/src/hw/pvscsi.c b/src/hw/pvscsi.c
index 8b4b10f..601a551 100644
--- a/src/hw/pvscsi.c
+++ b/src/hw/pvscsi.c
@@ -7,7 +7,6 @@
//
// This file may be distributed under the terms of the GNU LGPLv3 license.
-#include "biosvar.h" // GET_GLOBALFLAT
#include "block.h" // struct drive_s
#include "blockcmd.h" // scsi_drive_setup
#include "config.h" // CONFIG_*
@@ -16,11 +15,12 @@
#include "pci.h" // foreachpci
#include "pci_ids.h" // PCI_DEVICE_ID_VMWARE_PVSCSI
#include "pci_regs.h" // PCI_VENDOR_ID
+#include "pvscsi.h" // pvscsi_setup
#include "std/disk.h" // DISK_RET_SUCCESS
#include "string.h" // memset
#include "util.h" // usleep
-#include "pvscsi.h"
#include "virtio-ring.h" // PAGE_SHIFT, virt_to_phys
+#include "x86.h" // writel
#define MASK(n) ((1 << (n)) - 1)
@@ -159,7 +159,6 @@ pvscsi_wait_intr_cmpl(void *iobase)
while (!(readl(iobase + PVSCSI_REG_OFFSET_INTR_STATUS) & PVSCSI_INTR_CMPL_MASK))
usleep(5);
writel(iobase + PVSCSI_REG_OFFSET_INTR_STATUS, PVSCSI_INTR_CMPL_MASK);
-
}
static void
@@ -203,60 +202,58 @@ static void pvscsi_fill_req(struct PVSCSIRingsState *s,
u16 target, u16 lun, void *cdbcmd, u16 blocksize,
struct disk_op_s *op)
{
- SET_LOWFLAT(req->bus, 0);
- SET_LOWFLAT(req->target, target);
- memset(LOWFLAT2LOW(&req->lun[0]), 0, sizeof(req->lun));
- SET_LOWFLAT(req->lun[1], lun);
- SET_LOWFLAT(req->senseLen, 0);
- SET_LOWFLAT(req->senseAddr, 0);
- SET_LOWFLAT(req->cdbLen, 16);
- SET_LOWFLAT(req->vcpuHint, 0);
- memcpy(LOWFLAT2LOW(&req->cdb[0]), cdbcmd, 16);
- SET_LOWFLAT(req->tag, SIMPLE_QUEUE_TAG);
- SET_LOWFLAT(req->flags,
- cdb_is_read(cdbcmd, blocksize) ?
- PVSCSI_FLAG_CMD_DIR_TOHOST : PVSCSI_FLAG_CMD_DIR_TODEVICE);
-
- SET_LOWFLAT(req->dataLen, op->count * blocksize);
- SET_LOWFLAT(req->dataAddr, (u32)op->buf_fl);
- SET_LOWFLAT(s->reqProdIdx, GET_LOWFLAT(s->reqProdIdx) + 1);
-
+ req->bus = 0;
+ req->target = target;
+ memset(req->lun, 0, sizeof(req->lun));
+ req->lun[1] = lun;
+ req->senseLen = 0;
+ req->senseAddr = 0;
+ req->cdbLen = 16;
+ req->vcpuHint = 0;
+ memcpy(req->cdb, cdbcmd, 16);
+ req->tag = SIMPLE_QUEUE_TAG;
+ req->flags = cdb_is_read(cdbcmd, blocksize) ?
+ PVSCSI_FLAG_CMD_DIR_TOHOST : PVSCSI_FLAG_CMD_DIR_TODEVICE;
+
+ req->dataLen = op->count * blocksize;
+ req->dataAddr = (u32)op->buf_fl;
+ s->reqProdIdx = s->reqProdIdx + 1;
}
static u32
pvscsi_get_rsp(struct PVSCSIRingsState *s,
struct PVSCSIRingCmpDesc *rsp)
{
- u32 status = GET_LOWFLAT(rsp->hostStatus);
- SET_LOWFLAT(s->cmpConsIdx, GET_LOWFLAT(s->cmpConsIdx)+1);
+ u32 status = rsp->hostStatus;
+ s->cmpConsIdx = s->cmpConsIdx + 1;
return status;
}
static int
-pvscsi_cmd(struct pvscsi_lun_s *plun_gf, struct disk_op_s *op,
+pvscsi_cmd(struct pvscsi_lun_s *plun, struct disk_op_s *op,
void *cdbcmd, u16 target, u16 lun, u16 blocksize)
{
- struct pvscsi_ring_dsc_s *ring_dsc = GET_GLOBALFLAT(plun_gf->ring_dsc);
- struct PVSCSIRingsState *s = GET_LOWFLAT(ring_dsc->ring_state);
- u32 req_entries = GET_LOWFLAT(s->reqNumEntriesLog2);
- u32 cmp_entries = GET_LOWFLAT(s->cmpNumEntriesLog2);
+ struct pvscsi_ring_dsc_s *ring_dsc = plun->ring_dsc;
+ struct PVSCSIRingsState *s = ring_dsc->ring_state;
+ u32 req_entries = s->reqNumEntriesLog2;
+ u32 cmp_entries = s->cmpNumEntriesLog2;
struct PVSCSIRingReqDesc *req;
struct PVSCSIRingCmpDesc *rsp;
u32 status;
- if (GET_LOWFLAT(s->reqProdIdx) - GET_LOWFLAT(s->cmpConsIdx) >= 1 <<
req_entries) {
+ if (s->reqProdIdx - s->cmpConsIdx >= 1 << req_entries) {
dprintf(1, "pvscsi: ring full: reqProdIdx=%d cmpConsIdx=%d\n",
- GET_LOWFLAT(s->reqProdIdx), GET_LOWFLAT(s->cmpConsIdx));
+ s->reqProdIdx, s->cmpConsIdx);
return DISK_RET_EBADTRACK;
}
- req = GET_LOWFLAT(ring_dsc->ring_reqs) + (GET_LOWFLAT(s->reqProdIdx) &
MASK(req_entries));
+ req = ring_dsc->ring_reqs + (s->reqProdIdx & MASK(req_entries));
pvscsi_fill_req(s, req, target, lun, cdbcmd, blocksize, op);
- pvscsi_kick_rw_io(GET_GLOBALFLAT(plun_gf->iobase));
- pvscsi_wait_intr_cmpl(GET_GLOBALFLAT(plun_gf->iobase));
+ pvscsi_kick_rw_io(plun->iobase);
+ pvscsi_wait_intr_cmpl(plun->iobase);
- rsp = GET_LOWFLAT(ring_dsc->ring_cmps) + (GET_LOWFLAT(s->cmpConsIdx) &
MASK(cmp_entries));
+ rsp = ring_dsc->ring_cmps + (s->cmpConsIdx & MASK(cmp_entries));
status = pvscsi_get_rsp(s, rsp);
return status == 0 ? DISK_RET_SUCCESS : DISK_RET_EBADTRACK;
@@ -268,14 +265,10 @@ pvscsi_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize)
if (!CONFIG_PVSCSI)
return DISK_RET_EBADTRACK;
- struct pvscsi_lun_s *plun_gf =
+ struct pvscsi_lun_s *plun =
container_of(op->drive_gf, struct pvscsi_lun_s, drive);
- return pvscsi_cmd(plun_gf, op, cdbcmd,
- GET_GLOBALFLAT(plun_gf->target),
- GET_GLOBALFLAT(plun_gf->lun),
- blocksize);
-
+ return pvscsi_cmd(plun, op, cdbcmd, plun->target, plun->lun, blocksize);
}
static int
@@ -340,7 +333,6 @@ init_pvscsi(struct pci_device *pci)
pvscsi_scan_target(pci, iobase, ring_dsc, i);
return;
-
}
void
--
1.8.3.1
7 p.m.
On Fri, 27 Dec 2013 12:17:42 -0500
Kevin O'Connor <kevin(a)koconnor.net> wrote:
As with the recent change to AHCI, if the pvscsi driver needs to jump
into 32bit mode to access a register, then it's better to run the
pvscsi driver entirely in 32bit mode.
Evgeny, can you take a look at this and give it a test? Also, what
command line do you use for testing?
I tried testing locally by adding:
-device pvscsi,id=pvscsi0 -device scsi-disk,bus=pvscsi0.0,drive=drive0 -drive
id=drive0,if=none,file=dos-drivec-new
to my qemu command line (both qemu v1.6 and qemu v1.7), but it doesn't
work for me even before my changes. It hangs in
pvscsi_wait_intr_cmpl().
Try the following patch to qemu. I was getting hangs in this routine due to
language definition/compiler/bogus input issues. The old one returned an
off-by-one value.
diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index e35bff7..cfd7247 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -119,11 +119,8 @@ typedef struct PVSCSIRequest {
static int
pvscsi_log2(uint32_t input)
{
- int log = 0;
assert(input > 0);
- while (input >> ++log) {
- }
- return log;
+ return 31 - clz32(input);
}
static void
If this works, I'll pass it on to the qemu list.
-d
-Kevin
Kevin O'Connor (3):
pvscsi: Don't store reference to struct pci_device.
pvscsi: Always run entirely in 32bit mode.
pvscsi: Remove use of LOWFLAT and GLOBALFLAT macros.
Makefile | 4 +--
src/block.c | 8 +++--
src/hw/blockcmd.c | 3 +-
src/hw/pvscsi.c | 106 +++++++++++++++++++++++++-----------------------------
4 files changed, 58 insertions(+), 63 deletions(-)
--
1.8.3.1
_______________________________________________
SeaBIOS mailing list
SeaBIOS(a)seabios.org
http://www.seabios.org/mailman/listinfo/seabios
1:40 a.m.
On Wed, May 14, 2014 at 01:00:39PM -0400, Don Koch wrote:
On Fri, 27 Dec 2013 12:17:42 -0500
Kevin O'Connor <kevin(a)koconnor.net> wrote:
It still fails for me. I patched qemu and ran it with the
command-line above and it still causes a hang in SeaBIOS.
-Kevin
As with the recent change to AHCI, if the pvscsi
driver needs to jump
into 32bit mode to access a register, then it's better to run the
pvscsi driver entirely in 32bit mode.
Evgeny, can you take a look at this and give it a test? Also, what
command line do you use for testing?
I tried testing locally by adding:
-device pvscsi,id=pvscsi0 -device scsi-disk,bus=pvscsi0.0,drive=drive0 -drive
id=drive0,if=none,file=dos-drivec-new
to my qemu command line (both qemu v1.6 and qemu v1.7), but it doesn't
work for me even before my changes. It hangs in
pvscsi_wait_intr_cmpl().
Try the following patch to qemu. I was getting hangs in this routine due to
language definition/compiler/bogus input issues. The old one returned an
off-by-one value.
4 p.m.
On Wed, 14 May 2014 19:40:23 -0400
Kevin O'Connor <kevin(a)koconnor.net> wrote:
On Wed, May 14, 2014 at 01:00:39PM -0400, Don Koch
wrote:
Not sure if this is your problem, but we noted an issue with the generated code
for seabios which includes the build machine name; depending on the length of
the machine name, it changed the value in an uninitialized variable that caused
hanging (or not). Shorter names seem to be less problematic.
On Fri, 27 Dec 2013 12:17:42 -0500
Kevin O'Connor <kevin(a)koconnor.net> wrote:
It still fails for me. I patched qemu and ran it with the
command-line above and it still causes a hang in SeaBIOS. As with the recent change to AHCI, if the pvscsi
driver needs to jump
into 32bit mode to access a register, then it's better to run the
pvscsi driver entirely in 32bit mode.
Evgeny, can you take a look at this and give it a test? Also, what
command line do you use for testing?
I tried testing locally by adding:
-device pvscsi,id=pvscsi0 -device scsi-disk,bus=pvscsi0.0,drive=drive0 -drive
id=drive0,if=none,file=dos-drivec-new
to my qemu command line (both qemu v1.6 and qemu v1.7), but it doesn't
work for me even before my changes. It hangs in
pvscsi_wait_intr_cmpl().
Try the following patch to qemu. I was getting hangs in this routine due to
language definition/compiler/bogus input issues. The old one returned an
off-by-one value. -Kevin
Still researching.
-d
2062
days inactive
2216
days old
6 comments
2 participants
participants (2)
-
Don Koch -
Kevin O'Connor