Re: [Qemu-devel] Re: Help on "Could not initialize SDL - exiting"
On Mon, Jan 4, 2010 at 8:06 AM, Dejun.Liu wrote: > Hi > > yes, I hava find that OE build SDL not support x11 ,so hava the error > No available video device. > > thanks all these helps Glad you spot the error. Let us know how you solve it further. Or maybe you'll be kind enough and post your howto to Qemu wiki. -- regards, Mulyadi Santosa Freelance Linux trainer and consultant blog: the-hydra.blogspot.com training: mulyaditraining.blogspot.com
[Qemu-devel] [PATCH 1/6] PCI config space access overhaul
Different host buses may have different layouts for config space accessors.
The Mac U3 for example uses the following define to access Type 0 (directly
attached) devices:
#define MACRISC_CFA0(devfn, off)\
((1 << (unsigned int)PCI_SLOT(dev_fn)) \
| (((unsigned int)PCI_FUNC(dev_fn)) << 8) \
| (((unsigned int)(off)) & 0xFCUL))
Obviously, this is different from the encoding we interpret in qemu. In
order to let host controllers take some influence there, we can just
introduce a decoding function that takes whatever accessor we have and
decodes it into understandable fields.
To not touch all different code paths in qemu, I added this on top of
the existing config_reg element. In the future, we should work on gradually
moving references to config_reg to config_reg_dec and then phase out
config_reg.
This patch is the groundwork for Uninorth PCI config space fixes.
Signed-off-by: Alexander Graf
CC: Michael S. Tsirkin
CC: Benjamin Herrenschmidt
---
v1 -> v2:
- merge data access functions
- use decoding function for config space bits
- introduce encoding function for x86 style encodings
---
hw/apb_pci.c |1 +
hw/grackle_pci.c |1 +
hw/gt64xxx.c |1 +
hw/pci.h | 13 ++
hw/pci_host.c | 48 ++-
hw/pci_host.h | 16
hw/pci_host_template.h | 90 +---
hw/pci_host_template_all.h | 23 +++
hw/piix_pci.c |1 +
hw/ppc4xx_pci.c|1 +
hw/ppce500_pci.c |1 +
hw/unin_pci.c |1 +
12 files changed, 123 insertions(+), 74 deletions(-)
create mode 100644 hw/pci_host_template_all.h
diff --git a/hw/apb_pci.c b/hw/apb_pci.c
index f05308b..fedb84c 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -222,6 +222,7 @@ PCIBus *pci_apb_init(target_phys_addr_t special_base,
/* mem_data */
sysbus_mmio_map(s, 3, mem_base);
d = FROM_SYSBUS(APBState, s);
+pci_host_init(&d->host_state);
d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
pci_apb_set_irq, pci_pbm_map_irq, pic,
0, 32);
diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c
index ee4fed5..7feba63 100644
--- a/hw/grackle_pci.c
+++ b/hw/grackle_pci.c
@@ -88,6 +88,7 @@ PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic)
qdev_init_nofail(dev);
s = sysbus_from_qdev(dev);
d = FROM_SYSBUS(GrackleState, s);
+pci_host_init(&d->host_state);
d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
pci_grackle_set_irq,
pci_grackle_map_irq,
diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
index fb7f5bd..8cf93ca 100644
--- a/hw/gt64xxx.c
+++ b/hw/gt64xxx.c
@@ -1120,6 +1120,7 @@ PCIBus *pci_gt64120_init(qemu_irq *pic)
s = qemu_mallocz(sizeof(GT64120State));
s->pci = qemu_mallocz(sizeof(GT64120PCIState));
+pci_host_init(s->pci);
s->pci->bus = pci_register_bus(NULL, "pci",
pci_gt64120_set_irq, pci_gt64120_map_irq,
pic, 144, 4);
diff --git a/hw/pci.h b/hw/pci.h
index fd16460..cfaa0a9 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -10,10 +10,23 @@
/* PCI bus */
+typedef struct PCIAddress {
+PCIBus *domain;
+uint8_t bus;
+uint8_t slot;
+uint8_t fn;
+} PCIAddress;
+
#define PCI_DEVFN(slot, func) slot) & 0x1f) << 3) | ((func) & 0x07))
#define PCI_SLOT(devfn) (((devfn) >> 3) & 0x1f)
#define PCI_FUNC(devfn) ((devfn) & 0x07)
+static inline uint32_t pci_addr_to_devfn(PCIAddress *addr, uint32_t offset)
+{
+return ((addr->bus & 0xff) << 16) | (PCI_DEVFN(addr->slot, addr->fn) << 8)
|
+ offset;
+}
+
/* Class, Vendor and Device IDs from Linux's pci_ids.h */
#include "pci_ids.h"
diff --git a/hw/pci_host.c b/hw/pci_host.c
index eeb8dee..746ffc2 100644
--- a/hw/pci_host.c
+++ b/hw/pci_host.c
@@ -32,13 +32,6 @@ do { printf("pci_host_data: " fmt , ## __VA_ARGS__); } while
(0)
#define PCI_DPRINTF(fmt, ...)
#endif
-/*
- * PCI address
- * bit 16 - 24: bus number
- * bit 8 - 15: devfun number
- * bit 0 - 7: offset in configuration space of a given pci device
- */
-
/* the helper functio to get a PCIDeice* for a given pci address */
static inline PCIDevice *pci_dev_find_by_addr(PCIBus *bus, uint32_t addr)
{
@@ -56,7 +49,7 @@ void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val,
int len)
if (!pci_dev)
return;
-PCI_DPRINTF("%s: %s: addr=%02"PRIx32" val=%08"PRI32x" len=%d\n",
+PCI_DPRINTF("%s: %s: addr=%02"PRIx32" val=%08"PRIx32" len=%d\n",
__func__, pci_dev->name, config_addr, val, len);
pci_dev->config_write(pci_dev, config_addr, val, len);
}
@@ -89,7 +82,9 @@ static void pci_host_config_writel(void *o
[Qemu-devel] [PATCH 5/6] Make interrupts work
The interrupt code as is didn't really work for me. I couldn't even convince
Linux to take interrupt 9 in an interrupt-map.
So let's do this right. Let's map all PCI interrupts to 0x1b - 0x1e. That way
we're at least a small step closer to what real hardware does.
I also took the interrupt pin to line conversion from OpenBIOS, which at least
assures us we're compatible with our firmware :-).
A dump of the PCI interrupt-map from a U2 (iBook):
9000 ff97c528 0034 0001
d800 ff97c528 003f 0001
c000 ff97c528 001b 0001
c800 ff97c528 001c 0001
d000 ff97c528 001d 0001
Signed-off-by: Alexander Graf
---
hw/unin_pci.c | 14 +++---
1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/hw/unin_pci.c b/hw/unin_pci.c
index a3b0435..3fa7850 100644
--- a/hw/unin_pci.c
+++ b/hw/unin_pci.c
@@ -36,22 +36,30 @@
#define UNIN_DPRINTF(fmt, ...)
#endif
+static const int unin_irq_line[] = { 0x1b, 0x1c, 0x1d, 0x1e };
+
typedef struct UNINState {
SysBusDevice busdev;
PCIHostState host_state;
} UNINState;
-/* Don't know if this matches real hardware, but it agrees with OHW. */
static int pci_unin_map_irq(PCIDevice *pci_dev, int irq_num)
{
-return (irq_num + (pci_dev->devfn >> 3)) & 3;
+int retval;
+int devfn = pci_dev->devfn & 0x00FF;
+
+retval = (((devfn >> 11) & 0x1F) + irq_num) & 3;
+
+return retval;
}
static void pci_unin_set_irq(void *opaque, int irq_num, int level)
{
qemu_irq *pic = opaque;
-qemu_set_irq(pic[irq_num + 8], level);
+UNIN_DPRINTF("%s: setting INT %d = %d\n", __func__,
+ unin_irq_line[irq_num], level);
+qemu_set_irq(pic[unin_irq_line[irq_num]], level);
}
static void pci_unin_save(QEMUFile* f, void *opaque)
--
1.6.0.2
[Qemu-devel] [PATCH 2/6] Add config space conversion function for uni_north
As stated in the previous patch, the Uninorth PCI bridge requires different
layouts in its PCI config space accessors.
This patch introduces a conversion function that makes it compatible with
the way Linux accesses it.
I also kept an OpenBIOS compatibility hack in. I think it'd be better to
take small steps here and do the config space access rework in OpenBIOS
later on. When that's done we can remove that hack.
Signed-off-by: Alexander Graf
CC: Michael S. Tsirkin
CC: Benjamin Herrenschmidt
---
v1 -> v2:
- convert Uninorth to use config space decoder
---
hw/unin_pci.c | 31 +++
1 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/hw/unin_pci.c b/hw/unin_pci.c
index fdb9401..5e69725 100644
--- a/hw/unin_pci.c
+++ b/hw/unin_pci.c
@@ -75,6 +75,36 @@ static void pci_unin_reset(void *opaque)
{
}
+static void unin_decode_config_reg(PCIHostState *s, uint32_t config_reg,
+ PCIConfigAddress *decoded)
+{
+decoded->addr.domain = s->bus;
+decoded->addr_mask = 7;
+decoded->offset = config_reg & 0xfc;
+decoded->addr.fn = (config_reg >> 8) & 0x7;
+
+if (config_reg & (1u << 31)) {
+/* XXX OpenBIOS compatibility hack */
+pci_host_decode_config_reg(s, config_reg, decoded);
+} else if (config_reg & 1) {
+/* CFA1 style values */
+
+decoded->valid = (config_reg & 1) ? true : false;
+decoded->addr.bus = (config_reg >> 16) & 0xff;
+decoded->addr.slot = (config_reg >> 11) & 0x1f;
+} else {
+/* CFA0 style values */
+
+decoded->valid = true;
+decoded->addr.bus = 0;
+decoded->addr.slot = ffs(config_reg & 0xf800) - 1;
+}
+
+UNIN_DPRINTF("Converted config space accessor %08x -> %02x %02x %x\n",
+ config_reg, decoded->addr.bus, decoded->addr.slot,
+ decoded->addr.fn);
+}
+
static int pci_unin_main_init_device(SysBusDevice *dev)
{
UNINState *s;
@@ -85,6 +115,7 @@ static int pci_unin_main_init_device(SysBusDevice *dev)
s = FROM_SYSBUS(UNINState, dev);
pci_host_init(&s->host_state);
+s->host_state.decode_config_reg = unin_decode_config_reg;
pci_mem_config = pci_host_conf_register_mmio(&s->host_state);
pci_mem_data = pci_host_data_register_mmio(&s->host_state);
sysbus_init_mmio(dev, 0x1000, pci_mem_config);
--
1.6.0.2
[Qemu-devel] [PATCH 3/6] Use Mac99_U3 type on ppc64
The "Mac99" type so far defines a "U2" based configuration. Unfortunately,
there have never been any U2 based PPC64 machines. That's what the U3 was
developed for.
So let's split the Mac99 machine in a PPC64 and a PPC32 machine. The PPC32
machine stays "Mac99", while the PPC64 one becomes "Mac99_U3". All peripherals
stay the same.
Signed-off-by: Alexander Graf
---
v1 -> v2:
- s/get_config/decode_config/
---
hw/pci_ids.h |1 +
hw/ppc.h |1 +
hw/ppc_mac.h |1 +
hw/ppc_newworld.c | 12 +++-
hw/unin_pci.c | 70 +
5 files changed, 83 insertions(+), 2 deletions(-)
diff --git a/hw/pci_ids.h b/hw/pci_ids.h
index 63379c2..fe7a121 100644
--- a/hw/pci_ids.h
+++ b/hw/pci_ids.h
@@ -63,6 +63,7 @@
#define PCI_VENDOR_ID_APPLE 0x106b
#define PCI_DEVICE_ID_APPLE_UNI_N_AGP0x0020
+#define PCI_DEVICE_ID_APPLE_U3_AGP 0x004b
#define PCI_VENDOR_ID_SUN0x108e
#define PCI_DEVICE_ID_SUN_EBUS 0x1000
diff --git a/hw/ppc.h b/hw/ppc.h
index bbf3a98..b9a12a1 100644
--- a/hw/ppc.h
+++ b/hw/ppc.h
@@ -40,6 +40,7 @@ enum {
ARCH_PREP = 0,
ARCH_MAC99,
ARCH_HEATHROW,
+ARCH_MAC99_U3,
};
#define FW_CFG_PPC_WIDTH (FW_CFG_ARCH_LOCAL + 0x00)
diff --git a/hw/ppc_mac.h b/hw/ppc_mac.h
index a04dffe..89f96bb 100644
--- a/hw/ppc_mac.h
+++ b/hw/ppc_mac.h
@@ -58,6 +58,7 @@ PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic);
/* UniNorth PCI */
PCIBus *pci_pmac_init(qemu_irq *pic);
+PCIBus *pci_pmac_u3_init(qemu_irq *pic);
/* Mac NVRAM */
typedef struct MacIONVRAMState MacIONVRAMState;
diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c
index a4c714a..308e102 100644
--- a/hw/ppc_newworld.c
+++ b/hw/ppc_newworld.c
@@ -114,6 +114,7 @@ static void ppc_core99_init (ram_addr_t ram_size,
void *fw_cfg;
void *dbdma;
uint8_t *vga_bios_ptr;
+int machine_arch;
linux_boot = (kernel_filename != NULL);
@@ -317,7 +318,14 @@ static void ppc_core99_init (ram_addr_t ram_size,
}
}
pic = openpic_init(NULL, &pic_mem_index, smp_cpus, openpic_irqs, NULL);
-pci_bus = pci_pmac_init(pic);
+if (PPC_INPUT(env) == PPC_FLAGS_INPUT_970) {
+/* 970 gets a U3 bus */
+pci_bus = pci_pmac_u3_init(pic);
+machine_arch = ARCH_MAC99_U3;
+} else {
+pci_bus = pci_pmac_init(pic);
+machine_arch = ARCH_MAC99;
+}
/* init basic PC hardware */
pci_vga_init(pci_bus, vga_bios_offset, vga_bios_size);
@@ -364,7 +372,7 @@ static void ppc_core99_init (ram_addr_t ram_size,
fw_cfg = fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
-fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, ARCH_MAC99);
+fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, machine_arch);
fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, kernel_base);
fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size);
if (kernel_cmdline) {
diff --git a/hw/unin_pci.c b/hw/unin_pci.c
index 5e69725..214ae3c 100644
--- a/hw/unin_pci.c
+++ b/hw/unin_pci.c
@@ -142,6 +142,27 @@ static int pci_dec_21154_init_device(SysBusDevice *dev)
return 0;
}
+static int pci_u3_agp_init_device(SysBusDevice *dev)
+{
+UNINState *s;
+int pci_mem_config, pci_mem_data;
+
+/* Uninorth U3 AGP bus */
+s = FROM_SYSBUS(UNINState, dev);
+
+pci_host_init(&s->host_state);
+s->host_state.decode_config_reg = unin_decode_config_reg;
+pci_mem_config = pci_host_conf_register_mmio(&s->host_state);
+pci_mem_data = pci_host_data_register_mmio(&s->host_state);
+sysbus_init_mmio(dev, 0x1000, pci_mem_config);
+sysbus_init_mmio(dev, 0x1000, pci_mem_data);
+
+register_savevm("uninorth", 0, 1, pci_unin_save, pci_unin_load,
&s->host_state);
+qemu_register_reset(pci_unin_reset, &s->host_state);
+
+return 0;
+}
+
static int pci_unin_agp_init_device(SysBusDevice *dev)
{
UNINState *s;
@@ -223,6 +244,31 @@ PCIBus *pci_pmac_init(qemu_irq *pic)
return d->host_state.bus;
}
+PCIBus *pci_pmac_u3_init(qemu_irq *pic)
+{
+DeviceState *dev;
+SysBusDevice *s;
+UNINState *d;
+
+/* Uninorth AGP bus */
+
+dev = qdev_create(NULL, "u3-agp");
+qdev_init_nofail(dev);
+s = sysbus_from_qdev(dev);
+d = FROM_SYSBUS(UNINState, s);
+
+d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
+ pci_unin_set_irq, pci_unin_map_irq,
+ pic, 11 << 3, 4);
+
+sysbus_mmio_map(s, 0, 0xf080);
+sysbus_mmio_map(s, 1, 0xf0c0);
+
+pci_create_simple(d->host_state.bus, 11 << 3, "u3-agp");
+
+return d->host_state.bus;
+}
+
static int unin_main_pci_host_init(PCIDevice *d)
{
pci_config_set_vendor_id(d->config, PCI_VENDOR_ID_APPLE);
@@ -278,6 +324,21 @@ static int unin_agp_pci_host_init(PCIDevice *d)
return 0
[Qemu-devel] [PATCH 6/6] Enable secondary cmd64x
We're not using any macio IDE devices, so let's enable the secondary cmd64x IDE device, so we get all four possible IDE devices exposed to the guest. Later we definitely need to enable macio or any other device that Linux understands in default configurations. Signed-off-by: Alexander Graf --- hw/ppc_newworld.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c index 308e102..d66860b 100644 --- a/hw/ppc_newworld.c +++ b/hw/ppc_newworld.c @@ -343,7 +343,7 @@ static void ppc_core99_init (ram_addr_t ram_size, hd[i] = drive_get(IF_IDE, i / MAX_IDE_DEVS, i % MAX_IDE_DEVS); } dbdma = DBDMA_init(&dbdma_mem_index); -pci_cmd646_ide_init(pci_bus, hd, 0); +pci_cmd646_ide_init(pci_bus, hd, 1); /* cuda also initialize ADB */ cuda_init(&cuda_mem_index, pic[0x19]); -- 1.6.0.2
[Qemu-devel] [PATCH 4/6] Include dump of lspci -nn on real G5
To ease debugging and to know what we're lacking, I found it really useful to
have an lspci dump of a real U3 based G5 around. So I added a comment for it.
If people don't think it's important enough to include this information in the
sources, just don't apply this patch.
Signed-off-by: Alexander Graf
---
hw/unin_pci.c | 25 +
1 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/hw/unin_pci.c b/hw/unin_pci.c
index 214ae3c..a3b0435 100644
--- a/hw/unin_pci.c
+++ b/hw/unin_pci.c
@@ -244,6 +244,31 @@ PCIBus *pci_pmac_init(qemu_irq *pic)
return d->host_state.bus;
}
+/*
+ * PCI bus layout on a real G5 (U3 based):
+ *
+ * :f0:0b.0 Host bridge [0600]: Apple Computer Inc. U3 AGP [106b:004b]
+ * :f0:10.0 VGA compatible controller [0300]: ATI Technologies Inc RV350
AP [Radeon 9600] [1002:4150]
+ * 0001:00:00.0 Host bridge [0600]: Apple Computer Inc. CPC945 HT Bridge
[106b:004a]
+ * 0001:00:01.0 PCI bridge [0604]: Advanced Micro Devices [AMD] AMD-8131 PCI-X
Bridge [1022:7450] (rev 12)
+ * 0001:00:02.0 PCI bridge [0604]: Advanced Micro Devices [AMD] AMD-8131 PCI-X
Bridge [1022:7450] (rev 12)
+ * 0001:00:03.0 PCI bridge [0604]: Apple Computer Inc. K2 HT-PCI Bridge
[106b:0045]
+ * 0001:00:04.0 PCI bridge [0604]: Apple Computer Inc. K2 HT-PCI Bridge
[106b:0046]
+ * 0001:00:05.0 PCI bridge [0604]: Apple Computer Inc. K2 HT-PCI Bridge
[106b:0047]
+ * 0001:00:06.0 PCI bridge [0604]: Apple Computer Inc. K2 HT-PCI Bridge
[106b:0048]
+ * 0001:00:07.0 PCI bridge [0604]: Apple Computer Inc. K2 HT-PCI Bridge
[106b:0049]
+ * 0001:01:07.0 Class [ff00]: Apple Computer Inc. K2 KeyLargo Mac/IO
[106b:0041] (rev 20)
+ * 0001:01:08.0 USB Controller [0c03]: Apple Computer Inc. K2 KeyLargo USB
[106b:0040]
+ * 0001:01:09.0 USB Controller [0c03]: Apple Computer Inc. K2 KeyLargo USB
[106b:0040]
+ * 0001:02:0b.0 USB Controller [0c03]: NEC Corporation USB [1033:0035] (rev 43)
+ * 0001:02:0b.1 USB Controller [0c03]: NEC Corporation USB [1033:0035] (rev 43)
+ * 0001:02:0b.2 USB Controller [0c03]: NEC Corporation USB 2.0 [1033:00e0]
(rev 04)
+ * 0001:03:0d.0 Class [ff00]: Apple Computer Inc. K2 ATA/100 [106b:0043]
+ * 0001:03:0e.0 FireWire (IEEE 1394) [0c00]: Apple Computer Inc. K2 FireWire
[106b:0042]
+ * 0001:04:0f.0 Ethernet controller [0200]: Apple Computer Inc. K2 GMAC (Sun
GEM) [106b:004c]
+ * 0001:05:0c.0 IDE interface [0101]: Broadcom K2 SATA [1166:0240]
+ *
+ */
PCIBus *pci_pmac_u3_init(qemu_irq *pic)
{
DeviceState *dev;
--
1.6.0.2
[Qemu-devel] [PATCH 0/6] PPC NewWorld fixery v2
I'm trying to get the PPC64 system emulation target working finally. While doing so, I ran into several issues, all related to PCI this time. This patchset fixes all the PCI config space access and PCI interrupt mapping issues I've found on PPC64. Using this and a patched OpenBIOS version, I can successfully access IDE devices and was booting a guest into the shell from IDE using serial console. To leverage this patch, you also need a few patches to OpenBIOS. I'll present them to the OpenBIOS list, but in general getting patches into Qemu is harder than getting them into OpenBIOS. So I want to wait for the rewiew process here first. Find the OpenBIOS patch at: http://alex.csgraf.de/openbios-ppc-u3.patch v1 -> v2: - use decoding function for config space bits - merge config space data access functions - introduce encoding function for x86 style encodings - convert Uninorth to use config space decoder Alexander Graf (6): PCI config space access overhaul Add config space conversion function for uni_north Use Mac99_U3 type on ppc64 Include dump of lspci -nn on real G5 Make interrupts work Enable secondary cmd64x hw/apb_pci.c |1 + hw/grackle_pci.c |1 + hw/gt64xxx.c |1 + hw/pci.h | 13 hw/pci_host.c | 48 --- hw/pci_host.h | 16 + hw/pci_host_template.h | 90 hw/pci_host_template_all.h | 23 +++ hw/pci_ids.h |1 + hw/piix_pci.c |1 + hw/ppc.h |1 + hw/ppc4xx_pci.c|1 + hw/ppc_mac.h |1 + hw/ppc_newworld.c | 14 - hw/ppce500_pci.c |1 + hw/unin_pci.c | 141 +++- 16 files changed, 274 insertions(+), 80 deletions(-) create mode 100644 hw/pci_host_template_all.h
[Qemu-devel] Re: [PATCH v2] virtio-blk physical block size
On 01/04/2010 05:08 AM, Rusty Russell wrote: On Tue, 29 Dec 2009 03:09:23 am Avi Kivity wrote: This patch adds a physical block size attribute to virtio disks, corresponding to /sys/devices/.../physical_block_size. It is defined as the request alignment which will not trigger RMW cycles. This can be important for modern disks which use 4K physical sectors (though they still support 512 logical sectors), and for file-backed disk images (which have both the underlying filesystem block size and their own allocation granularity to consider). Installers use this to align partitions to physical block boundaries. Note the spec already defined blk_size as the performance rather than minimum alignment. However the driver interpreted this as the logical block size, so I updated the spec to match the driver assuming the driver predates the spec and that this is an error. I thought this was what I was doing, but I have shown over and over that I have no idea about block devices. Our current driver treats BLK_SIZE as the logical and physical size (see blk_queue_logical_block_size). But we want them to be different. I have no idea what "logical" vs. "physical" actually means. Anyone? Most importantly, is it some Linux-internal difference or a real I/O-visible distinction? Yes. Logical block size is the minimum block size the hardware will allow. Try to write less than that, and the hardware will laugh in your face. Physical block size is the what the logical block size would have been is software didn't suck. In theory they should be the same, but since compatibility reaons clamp the logical block size to 512, they have to differ. A disk may have a physical block size of 4096 and emulate logical block size of 512 on top of that using read-modify-write. Or so I understand it. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
[Qemu-devel] Can anybody simply explains how the device model gets run?
As a learner of qemu, I am a little bit confused about the qemu device model. How the device model gets run and when the device interrupt is raised to CPU? In the cpu_exec loop or before it? Thanks
[Qemu-devel] Re: [PATCH v2] virtio-blk physical block size
On Tue, 29 Dec 2009 03:09:23 am Avi Kivity wrote: > This patch adds a physical block size attribute to virtio disks, > corresponding to /sys/devices/.../physical_block_size. It is defined as > the request alignment which will not trigger RMW cycles. This can be > important for modern disks which use 4K physical sectors (though they > still support 512 logical sectors), and for file-backed disk images (which > have both the underlying filesystem block size and their own allocation > granularity to consider). > > Installers use this to align partitions to physical block boundaries. > > Note the spec already defined blk_size as the performance rather than > minimum alignment. However the driver interpreted this as the logical > block size, so I updated the spec to match the driver assuming the driver > predates the spec and that this is an error. I thought this was what I was doing, but I have shown over and over that I have no idea about block devices. Our current driver treats BLK_SIZE as the logical and physical size (see blk_queue_logical_block_size). I have no idea what "logical" vs. "physical" actually means. Anyone? Most importantly, is it some Linux-internal difference or a real I/O-visible distinction? Rusty.
[Qemu-devel] Re: [PATCH 1/6] Make config space accessor host bus trapable
On 03.01.2010, at 21:50, Benjamin Herrenschmidt wrote: > On Sun, 2010-01-03 at 21:27 +0100, Alexander Graf wrote: > >> I think if unin_pci is the only user, it'd be better to do it hacky >> inside unin_pci.c. But if there's a chance there's another user, it'd >> be better to make it generic. >> >> Since this is the first time I ever stumbled across type 0 and type 1 >> PCI config space addresses, I simply don't know if there are any. Blue >> Swirls comment indicated that there are. Ben also sounded as if it's >> rather common to not use the x86 format. On the other hand, it looks >> like nobody in qemu needed it so far - and we're implementing ARM, >> MIPS and all other sorts of platforms. >> >> So if anyone with knowledge in PCI could shed some light here, please >> do so. > > My feeling is that what you're better off doing is to have the qemu core > take an abstract struct to identify a device config space location, that > consists of separate fields for: > > - The PCI domain (which is what host bridge it hangs off since bus > numbers are not unique between domains) > > - The bus number Hm, I think it'd make more sense to just store a PCIBus pointer in there. We could then fetch the bus and domain id from there. I'll write something up :-). Alex
[Qemu-devel] Re: [PATCH-RFC 0/3] qemu: memory barriers in virtio
On Thu, 24 Dec 2009 03:06:00 am Michael S. Tsirkin wrote: > On Wed, Dec 23, 2009 at 05:04:19PM +1030, Rusty Russell wrote: > > It's possible, but I don't know of any missing cases. Certainly *lguest* i > > is missing barriers, since it's UP, but the core virtio should have them. > > Something that Paul Brook pointed out, is that > using a 16 bit value in C like we do in guest, e.g. with > ring->avail.idx > might in theory result in two single byte reads. > > If this happens, guest will see a wrong index value. In the Linux kernel we make atomicity assumptions about fundamental types. (Specifically pointers). QEMU may not want to rely on such assumptions however, and make them explicit. I have sympathy with Paul here. Cheers, Rusty.
Re: [Qemu-devel] Re: Help on "Could not initialize SDL - exiting"
Hi
yes, I hava find that OE build SDL not support x11 ,so hava the error
No available video device.
thanks all these helps
Steven Liu
On Thu, 2009-12-31 at 13:30 +0100, Andreas Färber wrote:
> Hi,
>
> Am 31.12.2009 um 09:45 schrieb Dejun.Liu:
>
> > r...@dejunliu-desktop:/home/protocol/study/OE/build# qemu-system-arm
> > -M
> > versatilepb -usb -usbdevice wacom-tablet -show-cursor -m 64
> > -kernel /home/protocol/study/OE/angstrom-dev/deploy/glibc/images/
> > qemuarm/zImage-2.6.25+2.6.26-rc4-r6-qemuarm.bin -drive file=/home/
> > protocol/study/OE/angstrom-dev/deploy/glibc/images/qemuarm/Angstrom-
> > x11-image-glibc-ipk-2009.X-stable-qemuarm.rootfs.ext3 -append "root=/
> > dev/sda"
> > Could not initialize SDL - exiting
> > SDL error: No available video device
>
> I had similar SDL issues on Fedora 12 ppc64. I ran qemu through
> 'strace', which showed me that some apparently dynamically loaded X11
> libraries were not installed by default.
>
> Andreas
>
> > How should i do?
> >
> > Steven Liu
> >
> > On Wed, 2009-12-30 at 11:31 +0200, Michael S. Tsirkin wrote:
> >> On Sun, Dec 27, 2009 at 05:35:56AM -0300, Dejun.Liu wrote:
> >>> Hi,
> >>>
> >>> below is my DISPLAY info:
> >>> echo ${DISPLAY}
> >>> :0.0
> >>>
> >>> and i have already added localhost to xhost with the command:
> >>> xhost localhost
> >>>
> >>> but the error continue the same.
> >>>
> >>> r...@dejunliu-desktop:~# echo ${DISPLAY}
> >>> :0.0
> >>> r...@dejunliu-desktop:~# xhost localhost
> >>> localhost being added to access control list
> >>> r...@dejunliu-desktop:~# qemu-system-arm -M versatilepb -usb -
> >>> usbdevice
> >>> wacom-tablet -show-cursor -m 64
> >>> -kernel /home/protocol/study/OE/angstrom-dev/deploy/glibc/images/
> >>> qemuarm/zImage-2.6.25+2.6.26-rc4-r6-qemuarm.bin -drive file=/home/
> >>> protocol/study/OE/angstrom-dev/deploy/glibc/images/qemuarm/
> >>> Angstrom-x11-image-glibc-ipk-2009.X-stable-qemuarm.rootfs.ext3 -
> >>> append "root=/dev/sda"
> >>> Could not initialize SDL - exiting
> >>>
> >>> so ,is my configure right?
> >>>
> >>> cheers
> >>>
> >>> Steven Liu
> >>>
> >>>
> >>> On Tue, 2009-12-29 at 12:18 +0200, Michael S. Tsirkin wrote:
> No, I mean DISPLAY.
>
> On Sat, Dec 26, 2009 at 02:44:05PM -0300, Dejun.Liu wrote:
> > Hi,
> >
> > do u mean:
> > export SDL_VIDEODRIVER=x11
> > ?
> > i hava set sdl video driver to x11.
> >
> > Steven Liu
> >
> > On Tue, 2009-12-29 at 12:04 +0200, Michael S. Tsirkin wrote:
> >> Most likely DISPLAY not set.
> >>
> >> On Sat, Dec 26, 2009 at 07:40:01AM -0300, Dejun.Liu wrote:
> >>> Hi,
> >>>
> >>> Im a newbie to qemu use.
> >>> so i got a lot error!..
> >>>
> >>> I build qemu from source with the version 0.10.3,
> >>>
> >>> But when i start qemu with the command below, i got the message
> >>> Could not initialize SDL - exiting
> >>>
> >>> after that Qemu exit. I dig the web ,but i could not find any
> >>> answers,so
> >>> I ask this question in this maillist,sorry for this.
> >>>
> >>> Command:
> >>> /angstrom-dev/staging/i686-linux/usr/bin/qemu-system-arm -M
> >>> versatilepb -usb -usbdevice wacom-tablet -show-cursor -m 64
> >>> -kernel /home/protocol/study/OE/angstrom-dev/deploy/glibc/
> >>> images/qemuarm/zImage-2.6.25+2.6.26-rc4-r6-qemuarm.bin -drive
> >>> file=/home/protocol/study/OE/angstrom-dev/deploy/glibc/images/
> >>> qemuarm/Angstrom-x11-image-glibc-ipk-2009.X-stable-
> >>> qemuarm.rootfs.ext3 -append "root=/dev/sda"
> >>>
> >>> And ldd result of qemu-system-arm is:
> >>>
> >>> r...@dejunliu-desktop:~# ldd `which qemu-system-arm`
> >>> linux-gate.so.1 => (0xb7f4c000)
> >>> libm.so.6 => /lib/tls/i686/cmov/libm.so.6 (0xb7f18000)
> >>> libz.so.1
> >>> => /home/protocol/study/OE/angstrom-dev/staging/i686-linux/usr/
> >>> lib/libz.so.1 (0xb7f06000)
> >>> libpthread.so.0 => /lib/tls/i686/cmov/libpthread.so.0
> >>> (0xb7eed000)
> >>> librt.so.1 => /lib/tls/i686/cmov/librt.so.1 (0xb7ee4000)
> >>> libutil.so.1 => /lib/tls/i686/cmov/libutil.so.1 (0xb7ee)
> >>> libSDL-1.2.so.0
> >>> => /home/protocol/study/OE/angstrom-dev/staging/i686-linux/usr/
> >>> lib/libSDL-1.2.so.0 (0xb7e9f000)
> >>> libncurses.so.5
> >>> => /home/protocol/study/OE/angstrom-dev/staging/i686-linux/usr/
> >>> lib/libncurses.so.5 (0xb7e5c000)
> >>> libc.so.6 => /lib/tls/i686/cmov/libc.so.6 (0xb7d0c000)
> >>> /lib/ld-linux.so.2 (0xb7f4d000)
> >>> libdl.so.2 => /lib/tls/i686/cmov/libdl.so.2 (0xb7d08000)
> >>
> >> Oh, X is not linked to. If this is intentional, run with -curses
> >> flag,
> >> if not recompile qemu after installing X development libraries.
> >> You should be able to compile the following program without errors:
> >>
> >> #include
> >> #if defined(
[Qemu-devel] Re: [PATCH 1/6] Make config space accessor host bus trapable
On Sun, 2010-01-03 at 21:27 +0100, Alexander Graf wrote: > I think if unin_pci is the only user, it'd be better to do it hacky > inside unin_pci.c. But if there's a chance there's another user, it'd > be better to make it generic. > > Since this is the first time I ever stumbled across type 0 and type 1 > PCI config space addresses, I simply don't know if there are any. Blue > Swirls comment indicated that there are. Ben also sounded as if it's > rather common to not use the x86 format. On the other hand, it looks > like nobody in qemu needed it so far - and we're implementing ARM, > MIPS and all other sorts of platforms. > > So if anyone with knowledge in PCI could shed some light here, please > do so. My feeling is that what you're better off doing is to have the qemu core take an abstract struct to identify a device config space location, that consists of separate fields for: - The PCI domain (which is what host bridge it hangs off since bus numbers are not unique between domains) - The bus number - The device number (aka slot ID) - The function number Now, you can then provide a "helper" that translate the standard x86 type 0 and type 1 cf8/cfc accesses into the above for most bridges to use, and uninorth or other non-x86 that use different scheme can do their own decoding. The reason you want the above is to be more future proof, ie, you'll probably want to deal with PCIe extended function numbers for example, or implement different methods of MMCONFIG etc... and it's better to disconnect the low level decoding of the config space access from the internal representation in qemu. Cheers, Ben.
[Qemu-devel] Re: [PATCH 1/6] Make config space accessor host bus trapable
On 03.01.2010, at 19:06, Michael S. Tsirkin wrote:
> On Sun, Jan 03, 2010 at 06:50:15PM +0100, Alexander Graf wrote:
>>
>> On 03.01.2010, at 18:44, Michael S. Tsirkin wrote:
>>
>>> On Sun, Jan 03, 2010 at 06:40:52PM +0100, Alexander Graf wrote:
On 03.01.2010, at 18:29, Michael S. Tsirkin wrote:
> On Sun, Jan 03, 2010 at 05:09:32PM +0100, Alexander Graf wrote:
>>
>> On 03.01.2010, at 16:45, Michael S. Tsirkin wrote:
>>
>>> On Sun, Jan 03, 2010 at 02:50:45AM +0100, Alexander Graf wrote:
Different host buses may have different layouts for config space
accessors.
The Mac U3 for example uses the following define to access Type 0
(directly
attached) devices:
#define MACRISC_CFA0(devfn, off)\
((1 << (unsigned int)PCI_SLOT(dev_fn)) \
| (((unsigned int)PCI_FUNC(dev_fn)) << 8) \
| (((unsigned int)(off)) & 0xFCUL))
Obviously, this is different from the encoding we interpret in qemu. In
order to let host controllers take some influence there, we can just
introduce a converter function that takes whatever accessor we have and
makes a qemu understandable one out of it.
This patch is the groundwork for Uninorth PCI config space fixes.
Signed-off-by: Alexander Graf
CC: Michael S. Tsirkin
>>>
>>> Thanks!
>>>
>>> It always bothered me that the code in pci_host uses x86 encoding and
>>> other architectures are converted to x86. As long as we are adding
>>> redirection, maybe we should get rid of this, and make get_config_reg
>>> return register and devfn separately, so we don't need to encode/decode
>>> back and forth?
>>
>> Hmm, when touching code I'm not 100% sure what I'm doing I usually try
>> to build on stuff that is there already. That's exactly what happened
>> here.
>>
>> I'm personally not against defining the x86 format as qemu default. That
>> way everyone knows what to deal with. I'm not sure if PCI defines
>> anything that couldn't be represented by the x86 encoding in 32 bits. I
>> actually doubt it. So it seems like a pretty good fit, especially given
>> that all the other code is already in place.
>>
>>> OTOH if we are after a quick fix, won't it be simpler to have
>>> unin_pci simply use its own routines instead of
>>> pci_host_conf_register_mmio
>>> etc?
>>
>> Hm, I figured this is less work :-).
>
> Let me explain the idea: we have:
>
> static void pci_host_config_writel_ioport(void *opaque,
> uint32_t addr, uint32_t val)
> {
> PCIHostState *s = opaque;
>
> PCI_DPRINTF("%s addr %"PRIx32 " val %"PRIx32"\n", __func__, addr,
> val);
> s->config_reg = val;
> }
>
> static uint32_t pci_host_config_readl_ioport(void *opaque, uint32_t
> addr)
> {
> PCIHostState *s = opaque;
> uint32_t val = s->config_reg;
>
> PCI_DPRINTF("%s addr %"PRIx32" val %"PRIx32"\n", __func__, addr,
> val);
> return val;
> }
>
> void pci_host_conf_register_ioport(pio_addr_t ioport, PCIHostState *s)
> {
> register_ioport_write(ioport, 4, 4, pci_host_config_writel_ioport,
> s);
> register_ioport_read(ioport, 4, 4, pci_host_config_readl_ioport, s);
> }
>
> If instead of a simple config_reg = val we translate to pc format
> here, the rest will work. No?
Well, that'd mean I'd have to implement a config_reg read function and do
the conversion backwards again there. Or I could just save it off in the
unin state ... hmm ...
>>>
>>> Right.
>>>
Either way, let's better get this done right. I'd rather want to have a
proper framework in place in case someone else stumbles across something
similar.
Alex
>>>
>>> There's already ugliness with swap/noswap versions in there. Anything
>>> that claims to be a proper framework would need to address that as well
>>> IMO.
>>
>> Hm, so you'd rather wait for 5 years to have an all-in-one rework than
>> incrementally improving the existing code?
>
> Not really, incremental improvements are good. But what you posted does
> not seem to clean up most code, what it really does is fixes ppc
> unin_pci. My feeling was since there's only one user for now maybe it
> is better to just have device specific code rather than complicate
> generic code. Makes sense?
>
> We need to see several users before we add yet another level of
> indirection. If there is a level of indirection that solves the
> swap/noswap ugliness as well that would show this is a good abstraction.
> I think I even know what a good abstraction would be: decode address o
[Qemu-devel] Re: [PATCH 1/6] Make config space accessor host bus trapable
On Sun, Jan 3, 2010 at 6:06 PM, Michael S. Tsirkin wrote:
> On Sun, Jan 03, 2010 at 06:50:15PM +0100, Alexander Graf wrote:
>>
>> On 03.01.2010, at 18:44, Michael S. Tsirkin wrote:
>>
>> > On Sun, Jan 03, 2010 at 06:40:52PM +0100, Alexander Graf wrote:
>> >>
>> >> On 03.01.2010, at 18:29, Michael S. Tsirkin wrote:
>> >>
>> >>> On Sun, Jan 03, 2010 at 05:09:32PM +0100, Alexander Graf wrote:
>>
>> On 03.01.2010, at 16:45, Michael S. Tsirkin wrote:
>>
>> > On Sun, Jan 03, 2010 at 02:50:45AM +0100, Alexander Graf wrote:
>> >> Different host buses may have different layouts for config space
>> >> accessors.
>> >>
>> >> The Mac U3 for example uses the following define to access Type 0
>> >> (directly
>> >> attached) devices:
>> >>
>> >> #define MACRISC_CFA0(devfn, off) \
>> >> ((1 << (unsigned int)PCI_SLOT(dev_fn)) \
>> >> | (((unsigned int)PCI_FUNC(dev_fn)) << 8) \
>> >> | (((unsigned int)(off)) & 0xFCUL))
>> >>
>> >> Obviously, this is different from the encoding we interpret in qemu.
>> >> In
>> >> order to let host controllers take some influence there, we can just
>> >> introduce a converter function that takes whatever accessor we have
>> >> and
>> >> makes a qemu understandable one out of it.
>> >>
>> >> This patch is the groundwork for Uninorth PCI config space fixes.
>> >>
>> >> Signed-off-by: Alexander Graf
>> >> CC: Michael S. Tsirkin
>> >
>> > Thanks!
>> >
>> > It always bothered me that the code in pci_host uses x86 encoding and
>> > other architectures are converted to x86. As long as we are adding
>> > redirection, maybe we should get rid of this, and make get_config_reg
>> > return register and devfn separately, so we don't need to encode/decode
>> > back and forth?
>>
>> Hmm, when touching code I'm not 100% sure what I'm doing I usually try
>> to build on stuff that is there already. That's exactly what happened
>> here.
>>
>> I'm personally not against defining the x86 format as qemu default.
>> That way everyone knows what to deal with. I'm not sure if PCI defines
>> anything that couldn't be represented by the x86 encoding in 32 bits. I
>> actually doubt it. So it seems like a pretty good fit, especially given
>> that all the other code is already in place.
>>
>> > OTOH if we are after a quick fix, won't it be simpler to have
>> > unin_pci simply use its own routines instead of
>> > pci_host_conf_register_mmio
>> > etc?
>>
>> Hm, I figured this is less work :-).
>> >>>
>> >>> Let me explain the idea: we have:
>> >>>
>> >>> static void pci_host_config_writel_ioport(void *opaque,
>> >>> uint32_t addr, uint32_t val)
>> >>> {
>> >>> PCIHostState *s = opaque;
>> >>>
>> >>> PCI_DPRINTF("%s addr %"PRIx32 " val %"PRIx32"\n", __func__, addr,
>> >>> val);
>> >>> s->config_reg = val;
>> >>> }
>> >>>
>> >>> static uint32_t pci_host_config_readl_ioport(void *opaque, uint32_t
>> >>> addr)
>> >>> {
>> >>> PCIHostState *s = opaque;
>> >>> uint32_t val = s->config_reg;
>> >>>
>> >>> PCI_DPRINTF("%s addr %"PRIx32" val %"PRIx32"\n", __func__, addr,
>> >>> val);
>> >>> return val;
>> >>> }
>> >>>
>> >>> void pci_host_conf_register_ioport(pio_addr_t ioport, PCIHostState *s)
>> >>> {
>> >>> register_ioport_write(ioport, 4, 4, pci_host_config_writel_ioport,
>> >>> s);
>> >>> register_ioport_read(ioport, 4, 4, pci_host_config_readl_ioport,
>> >>> s);
>> >>> }
>> >>>
>> >>> If instead of a simple config_reg = val we translate to pc format
>> >>> here, the rest will work. No?
>> >>
>> >> Well, that'd mean I'd have to implement a config_reg read function and do
>> >> the conversion backwards again there. Or I could just save it off in the
>> >> unin state ... hmm ...
>> >
>> > Right.
>> >
>> >> Either way, let's better get this done right. I'd rather want to have a
>> >> proper framework in place in case someone else stumbles across something
>> >> similar.
>> >>
>> >> Alex
>> >
>> > There's already ugliness with swap/noswap versions in there. Anything
>> > that claims to be a proper framework would need to address that as well
>> > IMO.
>>
>> Hm, so you'd rather wait for 5 years to have an all-in-one rework than
>> incrementally improving the existing code?
>
> Not really, incremental improvements are good. But what you posted does
> not seem to clean up most code, what it really does is fixes ppc
> unin_pci. My feeling was since there's only one user for now maybe it
> is better to just have device specific code rather than complicate
> generic code. Makes sense?
>
> We need to see several users before we add yet another level of
> indirection. If there is a level of indirection that solves the
> swap/noswap ugliness as well that
[Qemu-devel] need help getting a USB audio device to work
After weeks of fruitless effort I could use some help getting a USB audio device to work. I have instrumented the hell out of the guest driver and uhci code, qemu's linux and uhci code, and the host side usb code. Near as I can tell data from the device makes its way into qemu (async_complete shows a urb length equal to the data the host OS receives from the device), but the data does not appear to make its way to the guest OS. I have tried a variety of guests -- Fedora 12, RHEL5.3, and RHEL3U8, and none work. How do I determine in fact the data pulled into qemu from ioctl(USBDEVFS_REAPURBNDELAY) is getting pushed to the guest? Thanks, -- David Ahern
[Qemu-devel] Re: [PATCH 1/6] Make config space accessor host bus trapable
On Sun, Jan 03, 2010 at 06:50:15PM +0100, Alexander Graf wrote:
>
> On 03.01.2010, at 18:44, Michael S. Tsirkin wrote:
>
> > On Sun, Jan 03, 2010 at 06:40:52PM +0100, Alexander Graf wrote:
> >>
> >> On 03.01.2010, at 18:29, Michael S. Tsirkin wrote:
> >>
> >>> On Sun, Jan 03, 2010 at 05:09:32PM +0100, Alexander Graf wrote:
>
> On 03.01.2010, at 16:45, Michael S. Tsirkin wrote:
>
> > On Sun, Jan 03, 2010 at 02:50:45AM +0100, Alexander Graf wrote:
> >> Different host buses may have different layouts for config space
> >> accessors.
> >>
> >> The Mac U3 for example uses the following define to access Type 0
> >> (directly
> >> attached) devices:
> >>
> >> #define MACRISC_CFA0(devfn, off)\
> >> ((1 << (unsigned int)PCI_SLOT(dev_fn)) \
> >> | (((unsigned int)PCI_FUNC(dev_fn)) << 8) \
> >> | (((unsigned int)(off)) & 0xFCUL))
> >>
> >> Obviously, this is different from the encoding we interpret in qemu. In
> >> order to let host controllers take some influence there, we can just
> >> introduce a converter function that takes whatever accessor we have and
> >> makes a qemu understandable one out of it.
> >>
> >> This patch is the groundwork for Uninorth PCI config space fixes.
> >>
> >> Signed-off-by: Alexander Graf
> >> CC: Michael S. Tsirkin
> >
> > Thanks!
> >
> > It always bothered me that the code in pci_host uses x86 encoding and
> > other architectures are converted to x86. As long as we are adding
> > redirection, maybe we should get rid of this, and make get_config_reg
> > return register and devfn separately, so we don't need to encode/decode
> > back and forth?
>
> Hmm, when touching code I'm not 100% sure what I'm doing I usually try
> to build on stuff that is there already. That's exactly what happened
> here.
>
> I'm personally not against defining the x86 format as qemu default. That
> way everyone knows what to deal with. I'm not sure if PCI defines
> anything that couldn't be represented by the x86 encoding in 32 bits. I
> actually doubt it. So it seems like a pretty good fit, especially given
> that all the other code is already in place.
>
> > OTOH if we are after a quick fix, won't it be simpler to have
> > unin_pci simply use its own routines instead of
> > pci_host_conf_register_mmio
> > etc?
>
> Hm, I figured this is less work :-).
> >>>
> >>> Let me explain the idea: we have:
> >>>
> >>> static void pci_host_config_writel_ioport(void *opaque,
> >>> uint32_t addr, uint32_t val)
> >>> {
> >>> PCIHostState *s = opaque;
> >>>
> >>> PCI_DPRINTF("%s addr %"PRIx32 " val %"PRIx32"\n", __func__, addr,
> >>> val);
> >>> s->config_reg = val;
> >>> }
> >>>
> >>> static uint32_t pci_host_config_readl_ioport(void *opaque, uint32_t
> >>> addr)
> >>> {
> >>> PCIHostState *s = opaque;
> >>> uint32_t val = s->config_reg;
> >>>
> >>> PCI_DPRINTF("%s addr %"PRIx32" val %"PRIx32"\n", __func__, addr,
> >>> val);
> >>> return val;
> >>> }
> >>>
> >>> void pci_host_conf_register_ioport(pio_addr_t ioport, PCIHostState *s)
> >>> {
> >>> register_ioport_write(ioport, 4, 4, pci_host_config_writel_ioport,
> >>> s);
> >>> register_ioport_read(ioport, 4, 4, pci_host_config_readl_ioport, s);
> >>> }
> >>>
> >>> If instead of a simple config_reg = val we translate to pc format
> >>> here, the rest will work. No?
> >>
> >> Well, that'd mean I'd have to implement a config_reg read function and do
> >> the conversion backwards again there. Or I could just save it off in the
> >> unin state ... hmm ...
> >
> > Right.
> >
> >> Either way, let's better get this done right. I'd rather want to have a
> >> proper framework in place in case someone else stumbles across something
> >> similar.
> >>
> >> Alex
> >
> > There's already ugliness with swap/noswap versions in there. Anything
> > that claims to be a proper framework would need to address that as well
> > IMO.
>
> Hm, so you'd rather wait for 5 years to have an all-in-one rework than
> incrementally improving the existing code?
Not really, incremental improvements are good. But what you posted does
not seem to clean up most code, what it really does is fixes ppc
unin_pci. My feeling was since there's only one user for now maybe it
is better to just have device specific code rather than complicate
generic code. Makes sense?
We need to see several users before we add yet another level of
indirection. If there is a level of indirection that solves the
swap/noswap ugliness as well that would show this is a good abstraction.
I think I even know what a good abstraction would be: decode address on
write and keep it in decoded form.
> I can do the hacky version then :-).
>
[Qemu-devel] Re: [PATCH 1/6] Make config space accessor host bus trapable
On 03.01.2010, at 18:44, Michael S. Tsirkin wrote:
> On Sun, Jan 03, 2010 at 06:40:52PM +0100, Alexander Graf wrote:
>>
>> On 03.01.2010, at 18:29, Michael S. Tsirkin wrote:
>>
>>> On Sun, Jan 03, 2010 at 05:09:32PM +0100, Alexander Graf wrote:
On 03.01.2010, at 16:45, Michael S. Tsirkin wrote:
> On Sun, Jan 03, 2010 at 02:50:45AM +0100, Alexander Graf wrote:
>> Different host buses may have different layouts for config space
>> accessors.
>>
>> The Mac U3 for example uses the following define to access Type 0
>> (directly
>> attached) devices:
>>
>> #define MACRISC_CFA0(devfn, off)\
>> ((1 << (unsigned int)PCI_SLOT(dev_fn)) \
>> | (((unsigned int)PCI_FUNC(dev_fn)) << 8) \
>> | (((unsigned int)(off)) & 0xFCUL))
>>
>> Obviously, this is different from the encoding we interpret in qemu. In
>> order to let host controllers take some influence there, we can just
>> introduce a converter function that takes whatever accessor we have and
>> makes a qemu understandable one out of it.
>>
>> This patch is the groundwork for Uninorth PCI config space fixes.
>>
>> Signed-off-by: Alexander Graf
>> CC: Michael S. Tsirkin
>
> Thanks!
>
> It always bothered me that the code in pci_host uses x86 encoding and
> other architectures are converted to x86. As long as we are adding
> redirection, maybe we should get rid of this, and make get_config_reg
> return register and devfn separately, so we don't need to encode/decode
> back and forth?
Hmm, when touching code I'm not 100% sure what I'm doing I usually try to
build on stuff that is there already. That's exactly what happened here.
I'm personally not against defining the x86 format as qemu default. That
way everyone knows what to deal with. I'm not sure if PCI defines anything
that couldn't be represented by the x86 encoding in 32 bits. I actually
doubt it. So it seems like a pretty good fit, especially given that all
the other code is already in place.
> OTOH if we are after a quick fix, won't it be simpler to have
> unin_pci simply use its own routines instead of
> pci_host_conf_register_mmio
> etc?
Hm, I figured this is less work :-).
>>>
>>> Let me explain the idea: we have:
>>>
>>> static void pci_host_config_writel_ioport(void *opaque,
>>> uint32_t addr, uint32_t val)
>>> {
>>> PCIHostState *s = opaque;
>>>
>>> PCI_DPRINTF("%s addr %"PRIx32 " val %"PRIx32"\n", __func__, addr,
>>> val);
>>> s->config_reg = val;
>>> }
>>>
>>> static uint32_t pci_host_config_readl_ioport(void *opaque, uint32_t
>>> addr)
>>> {
>>> PCIHostState *s = opaque;
>>> uint32_t val = s->config_reg;
>>>
>>> PCI_DPRINTF("%s addr %"PRIx32" val %"PRIx32"\n", __func__, addr,
>>> val);
>>> return val;
>>> }
>>>
>>> void pci_host_conf_register_ioport(pio_addr_t ioport, PCIHostState *s)
>>> {
>>> register_ioport_write(ioport, 4, 4, pci_host_config_writel_ioport,
>>> s);
>>> register_ioport_read(ioport, 4, 4, pci_host_config_readl_ioport, s);
>>> }
>>>
>>> If instead of a simple config_reg = val we translate to pc format
>>> here, the rest will work. No?
>>
>> Well, that'd mean I'd have to implement a config_reg read function and do
>> the conversion backwards again there. Or I could just save it off in the
>> unin state ... hmm ...
>
> Right.
>
>> Either way, let's better get this done right. I'd rather want to have a
>> proper framework in place in case someone else stumbles across something
>> similar.
>>
>> Alex
>
> There's already ugliness with swap/noswap versions in there. Anything
> that claims to be a proper framework would need to address that as well
> IMO.
Hm, so you'd rather wait for 5 years to have an all-in-one rework than
incrementally improving the existing code? I can do the hacky version then :-).
Alex
[Qemu-devel] Re: [PATCH 1/6] Make config space accessor host bus trapable
On Sun, Jan 03, 2010 at 06:40:52PM +0100, Alexander Graf wrote:
>
> On 03.01.2010, at 18:29, Michael S. Tsirkin wrote:
>
> > On Sun, Jan 03, 2010 at 05:09:32PM +0100, Alexander Graf wrote:
> >>
> >> On 03.01.2010, at 16:45, Michael S. Tsirkin wrote:
> >>
> >>> On Sun, Jan 03, 2010 at 02:50:45AM +0100, Alexander Graf wrote:
> Different host buses may have different layouts for config space
> accessors.
>
> The Mac U3 for example uses the following define to access Type 0
> (directly
> attached) devices:
>
> #define MACRISC_CFA0(devfn, off)\
> ((1 << (unsigned int)PCI_SLOT(dev_fn)) \
> | (((unsigned int)PCI_FUNC(dev_fn)) << 8) \
> | (((unsigned int)(off)) & 0xFCUL))
>
> Obviously, this is different from the encoding we interpret in qemu. In
> order to let host controllers take some influence there, we can just
> introduce a converter function that takes whatever accessor we have and
> makes a qemu understandable one out of it.
>
> This patch is the groundwork for Uninorth PCI config space fixes.
>
> Signed-off-by: Alexander Graf
> CC: Michael S. Tsirkin
> >>>
> >>> Thanks!
> >>>
> >>> It always bothered me that the code in pci_host uses x86 encoding and
> >>> other architectures are converted to x86. As long as we are adding
> >>> redirection, maybe we should get rid of this, and make get_config_reg
> >>> return register and devfn separately, so we don't need to encode/decode
> >>> back and forth?
> >>
> >> Hmm, when touching code I'm not 100% sure what I'm doing I usually try to
> >> build on stuff that is there already. That's exactly what happened here.
> >>
> >> I'm personally not against defining the x86 format as qemu default. That
> >> way everyone knows what to deal with. I'm not sure if PCI defines anything
> >> that couldn't be represented by the x86 encoding in 32 bits. I actually
> >> doubt it. So it seems like a pretty good fit, especially given that all
> >> the other code is already in place.
> >>
> >>> OTOH if we are after a quick fix, won't it be simpler to have
> >>> unin_pci simply use its own routines instead of
> >>> pci_host_conf_register_mmio
> >>> etc?
> >>
> >> Hm, I figured this is less work :-).
> >
> > Let me explain the idea: we have:
> >
> > static void pci_host_config_writel_ioport(void *opaque,
> > uint32_t addr, uint32_t val)
> > {
> > PCIHostState *s = opaque;
> >
> > PCI_DPRINTF("%s addr %"PRIx32 " val %"PRIx32"\n", __func__, addr,
> > val);
> > s->config_reg = val;
> > }
> >
> > static uint32_t pci_host_config_readl_ioport(void *opaque, uint32_t
> > addr)
> > {
> > PCIHostState *s = opaque;
> > uint32_t val = s->config_reg;
> >
> > PCI_DPRINTF("%s addr %"PRIx32" val %"PRIx32"\n", __func__, addr,
> > val);
> > return val;
> > }
> >
> > void pci_host_conf_register_ioport(pio_addr_t ioport, PCIHostState *s)
> > {
> > register_ioport_write(ioport, 4, 4, pci_host_config_writel_ioport,
> > s);
> > register_ioport_read(ioport, 4, 4, pci_host_config_readl_ioport, s);
> > }
> >
> > If instead of a simple config_reg = val we translate to pc format
> > here, the rest will work. No?
>
> Well, that'd mean I'd have to implement a config_reg read function and do the
> conversion backwards again there. Or I could just save it off in the unin
> state ... hmm ...
Right.
> Either way, let's better get this done right. I'd rather want to have a
> proper framework in place in case someone else stumbles across something
> similar.
>
> Alex
There's already ugliness with swap/noswap versions in there. Anything
that claims to be a proper framework would need to address that as well
IMO.
--
MST
[Qemu-devel] Re: [PATCH 1/6] Make config space accessor host bus trapable
On 03.01.2010, at 18:29, Michael S. Tsirkin wrote:
> On Sun, Jan 03, 2010 at 05:09:32PM +0100, Alexander Graf wrote:
>>
>> On 03.01.2010, at 16:45, Michael S. Tsirkin wrote:
>>
>>> On Sun, Jan 03, 2010 at 02:50:45AM +0100, Alexander Graf wrote:
Different host buses may have different layouts for config space accessors.
The Mac U3 for example uses the following define to access Type 0 (directly
attached) devices:
#define MACRISC_CFA0(devfn, off)\
((1 << (unsigned int)PCI_SLOT(dev_fn)) \
| (((unsigned int)PCI_FUNC(dev_fn)) << 8) \
| (((unsigned int)(off)) & 0xFCUL))
Obviously, this is different from the encoding we interpret in qemu. In
order to let host controllers take some influence there, we can just
introduce a converter function that takes whatever accessor we have and
makes a qemu understandable one out of it.
This patch is the groundwork for Uninorth PCI config space fixes.
Signed-off-by: Alexander Graf
CC: Michael S. Tsirkin
>>>
>>> Thanks!
>>>
>>> It always bothered me that the code in pci_host uses x86 encoding and
>>> other architectures are converted to x86. As long as we are adding
>>> redirection, maybe we should get rid of this, and make get_config_reg
>>> return register and devfn separately, so we don't need to encode/decode
>>> back and forth?
>>
>> Hmm, when touching code I'm not 100% sure what I'm doing I usually try to
>> build on stuff that is there already. That's exactly what happened here.
>>
>> I'm personally not against defining the x86 format as qemu default. That way
>> everyone knows what to deal with. I'm not sure if PCI defines anything that
>> couldn't be represented by the x86 encoding in 32 bits. I actually doubt it.
>> So it seems like a pretty good fit, especially given that all the other code
>> is already in place.
>>
>>> OTOH if we are after a quick fix, won't it be simpler to have
>>> unin_pci simply use its own routines instead of pci_host_conf_register_mmio
>>> etc?
>>
>> Hm, I figured this is less work :-).
>
> Let me explain the idea: we have:
>
> static void pci_host_config_writel_ioport(void *opaque,
> uint32_t addr, uint32_t val)
> {
> PCIHostState *s = opaque;
>
> PCI_DPRINTF("%s addr %"PRIx32 " val %"PRIx32"\n", __func__, addr,
> val);
> s->config_reg = val;
> }
>
> static uint32_t pci_host_config_readl_ioport(void *opaque, uint32_t
> addr)
> {
> PCIHostState *s = opaque;
> uint32_t val = s->config_reg;
>
> PCI_DPRINTF("%s addr %"PRIx32" val %"PRIx32"\n", __func__, addr,
> val);
> return val;
> }
>
> void pci_host_conf_register_ioport(pio_addr_t ioport, PCIHostState *s)
> {
> register_ioport_write(ioport, 4, 4, pci_host_config_writel_ioport,
> s);
> register_ioport_read(ioport, 4, 4, pci_host_config_readl_ioport, s);
> }
>
> If instead of a simple config_reg = val we translate to pc format
> here, the rest will work. No?
Well, that'd mean I'd have to implement a config_reg read function and do the
conversion backwards again there. Or I could just save it off in the unin state
... hmm ...
Either way, let's better get this done right. I'd rather want to have a proper
framework in place in case someone else stumbles across something similar.
Alex
[Qemu-devel] Re: [PATCH 1/6] Make config space accessor host bus trapable
On Sun, Jan 03, 2010 at 05:09:32PM +0100, Alexander Graf wrote:
>
> On 03.01.2010, at 16:45, Michael S. Tsirkin wrote:
>
> > On Sun, Jan 03, 2010 at 02:50:45AM +0100, Alexander Graf wrote:
> >> Different host buses may have different layouts for config space accessors.
> >>
> >> The Mac U3 for example uses the following define to access Type 0 (directly
> >> attached) devices:
> >>
> >> #define MACRISC_CFA0(devfn, off)\
> >>((1 << (unsigned int)PCI_SLOT(dev_fn)) \
> >>| (((unsigned int)PCI_FUNC(dev_fn)) << 8) \
> >>| (((unsigned int)(off)) & 0xFCUL))
> >>
> >> Obviously, this is different from the encoding we interpret in qemu. In
> >> order to let host controllers take some influence there, we can just
> >> introduce a converter function that takes whatever accessor we have and
> >> makes a qemu understandable one out of it.
> >>
> >> This patch is the groundwork for Uninorth PCI config space fixes.
> >>
> >> Signed-off-by: Alexander Graf
> >> CC: Michael S. Tsirkin
> >
> > Thanks!
> >
> > It always bothered me that the code in pci_host uses x86 encoding and
> > other architectures are converted to x86. As long as we are adding
> > redirection, maybe we should get rid of this, and make get_config_reg
> > return register and devfn separately, so we don't need to encode/decode
> > back and forth?
>
> Hmm, when touching code I'm not 100% sure what I'm doing I usually try to
> build on stuff that is there already. That's exactly what happened here.
>
> I'm personally not against defining the x86 format as qemu default. That way
> everyone knows what to deal with. I'm not sure if PCI defines anything that
> couldn't be represented by the x86 encoding in 32 bits. I actually doubt it.
> So it seems like a pretty good fit, especially given that all the other code
> is already in place.
>
> > OTOH if we are after a quick fix, won't it be simpler to have
> > unin_pci simply use its own routines instead of pci_host_conf_register_mmio
> > etc?
>
> Hm, I figured this is less work :-).
Let me explain the idea: we have:
static void pci_host_config_writel_ioport(void *opaque,
uint32_t addr, uint32_t val)
{
PCIHostState *s = opaque;
PCI_DPRINTF("%s addr %"PRIx32 " val %"PRIx32"\n", __func__, addr,
val);
s->config_reg = val;
}
static uint32_t pci_host_config_readl_ioport(void *opaque, uint32_t
addr)
{
PCIHostState *s = opaque;
uint32_t val = s->config_reg;
PCI_DPRINTF("%s addr %"PRIx32" val %"PRIx32"\n", __func__, addr,
val);
return val;
}
void pci_host_conf_register_ioport(pio_addr_t ioport, PCIHostState *s)
{
register_ioport_write(ioport, 4, 4, pci_host_config_writel_ioport,
s);
register_ioport_read(ioport, 4, 4, pci_host_config_readl_ioport, s);
}
If instead of a simple config_reg = val we translate to pc format
here, the rest will work. No?
> >
> >> ---
> >> hw/apb_pci.c |1 +
> >> hw/grackle_pci.c |1 +
> >> hw/gt64xxx.c |1 +
> >> hw/pci_host.c | 11 +++
> >> hw/pci_host.h |5 +
> >> hw/pci_host_template.h | 30 ++
> >> hw/piix_pci.c |1 +
> >> hw/ppc4xx_pci.c|1 +
> >> hw/ppce500_pci.c |1 +
> >> hw/unin_pci.c |1 +
> >> 10 files changed, 41 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/hw/apb_pci.c b/hw/apb_pci.c
> >> index f05308b..fedb84c 100644
> >> --- a/hw/apb_pci.c
> >> +++ b/hw/apb_pci.c
> >> @@ -222,6 +222,7 @@ PCIBus *pci_apb_init(target_phys_addr_t special_base,
> >> /* mem_data */
> >> sysbus_mmio_map(s, 3, mem_base);
> >> d = FROM_SYSBUS(APBState, s);
> >> +pci_host_init(&d->host_state);
> >> d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
> >> pci_apb_set_irq, pci_pbm_map_irq,
> >> pic,
> >> 0, 32);
> >> diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c
> >> index ee4fed5..7feba63 100644
> >> --- a/hw/grackle_pci.c
> >> +++ b/hw/grackle_pci.c
> >> @@ -88,6 +88,7 @@ PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic)
> >> qdev_init_nofail(dev);
> >> s = sysbus_from_qdev(dev);
> >> d = FROM_SYSBUS(GrackleState, s);
> >> +pci_host_init(&d->host_state);
> >> d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
> >> pci_grackle_set_irq,
> >> pci_grackle_map_irq,
> >> diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
> >> index fb7f5bd..8cf93ca 100644
> >> --- a/hw/gt64xxx.c
> >> +++ b/hw/gt64xxx.c
> >> @@ -1120,6 +1120,7 @@ PCIBus *pci_gt64120_init(qemu_irq *pic)
> >> s = qemu_mallocz(sizeof(GT64120State));
> >> s->p
[Qemu-devel] Re: [PATCH 2/6] Add config space conversion function for uni_north
On Sun, Jan 03, 2010 at 05:35:01PM +0100, Alexander Graf wrote:
>
> On 03.01.2010, at 17:20, Michael S. Tsirkin wrote:
>
> > On Sun, Jan 03, 2010 at 05:13:12PM +0100, Alexander Graf wrote:
> >>
> >> On 03.01.2010, at 16:47, Michael S. Tsirkin wrote:
> >>
> >>> On Sun, Jan 03, 2010 at 04:40:12PM +0100, Alexander Graf wrote:
>
> On 03.01.2010, at 16:32, Michael S. Tsirkin wrote:
>
> > On Sun, Jan 03, 2010 at 02:50:46AM +0100, Alexander Graf wrote:
> >> As stated in the previous patch, the Uninorth PCI bridge requires
> >> different
> >> layouts in its PCI config space accessors.
> >>
> >> This patch introduces a conversion function that makes it compatible
> >> with
> >> the way Linux accesses it.
> >>
> >> I also kept an OpenBIOS compatibility hack in. I think it'd be better
> >> to
> >> take small steps here and do the config space access rework in OpenBIOS
> >> later on. When that's done we can remove that hack.
> >>
> >> Signed-off-by: Alexander Graf
> >> ---
> >> hw/unin_pci.c | 35 +++
> >> 1 files changed, 35 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/hw/unin_pci.c b/hw/unin_pci.c
> >> index fdb9401..1c49008 100644
> >> --- a/hw/unin_pci.c
> >> +++ b/hw/unin_pci.c
> >> @@ -75,6 +75,40 @@ static void pci_unin_reset(void *opaque)
> >> {
> >> }
> >>
> >> +static uint32_t unin_get_config_reg(PCIHostState *s, uint32_t addr)
> >> +{
> >> +uint32_t retval;
> >> +uint32_t reg = s->config_reg;
> >> +
> >> +if (reg & (1u << 31)) {
> >> +/* XXX OpenBIOS compatibility hack */
> >> +retval = reg;
> >> +addr |= reg & 7;
> >> +} else if (reg & 1) {
> >> +/* Set upper valid bit and remove lower one */
> >> +retval = (reg & ~3u) | (1u << 31);
> >> +} else {
> >> +uint32_t slot, func;
> >> +uint32_t devfn;
> >> +
> >> +/* Grab CFA0 style values */
> >> +slot = ffs(reg & 0xf800) - 1;
> >> +func = (reg >> 8) & 7;
> >> +devfn = PCI_DEVFN(slot, func);
> >> +
> >> +/* ... and then convert them to x86 format */
> >> +retval = (reg & 0xfc) | (devfn << 8) | (1u << 31);
> >
> > Is it a good idea to have a helper that encodes reg/dev/fn into a 32 bit
> > number? This way this encoding can be changed down the road.
>
> I don't think I understand this comment? :-)
> >>>
> >>> This puts reg+dev+fn in a format that pci_host can the understand
> >>> correct? So it would make sense to have an inline function
> >>> in pci host that gets 3 parameters and does the encoding.
> >>
> >> We're doing the reverse here. We get a uint32_t (host->config_reg) and
> >> need to do something with it.
> >>
> >> We could either call a helper that splits it into bus,dev,fn or we could
> >> just put all of them into a single uint32_t again that later on gets
> >> interpreted in a specified format.
> >>
> >> I figured I'd have to touch less code and keep things more stable for the
> >> other (non-uninorth) buses if I keep the x86 format as "default format"
> >> and just convert to it. Passing a uint32_t is also easier than passing 3
> >> ints :-).
> >>
> >> Alex
> >
> > So what the comment above suggests, is adding helper routine
> > that gets register device, function and creates 32 bit value
> > in "default format".
>
> Oh, so you mean that instead of returning a uint32_t that magically gets
> converted inside the conversion function, we'd create another function like
> this:
>
> uint32_t pci_host_config_address(int bus, int dev, int fn)
> {
> return (1u << 31) | (bus << 11) | (dev << 3) | fn;
> }
>
> which then would be called at the end of the conversion function?
>
>
> Alex
Yes.
Re: [Qemu-devel] Trouble getting Ubuntu 64-bit working
On Sat, Jan 2, 2010 at 3:51 PM, Nathan Osman wrote: > I recently downloaded the qemu binary for Windows. I'm using Vista 32 bit as > the host and I'm trying to run Ubuntu 64-bit in it. The splash screen works, > but when it boots, I am left staring at a blank screen. I let it sit for > half an hour before giving up. Am I doing something wrong? I launched the > qemu-system-x86_64 exec. Could you provide us with the exact command line you use to start the ubuntu instance? -- regards, Mulyadi Santosa Freelance Linux trainer and consultant blog: the-hydra.blogspot.com training: mulyaditraining.blogspot.com
[Qemu-devel] Re: [PATCH 2/6] Add config space conversion function for uni_north
On 03.01.2010, at 17:20, Michael S. Tsirkin wrote:
> On Sun, Jan 03, 2010 at 05:13:12PM +0100, Alexander Graf wrote:
>>
>> On 03.01.2010, at 16:47, Michael S. Tsirkin wrote:
>>
>>> On Sun, Jan 03, 2010 at 04:40:12PM +0100, Alexander Graf wrote:
On 03.01.2010, at 16:32, Michael S. Tsirkin wrote:
> On Sun, Jan 03, 2010 at 02:50:46AM +0100, Alexander Graf wrote:
>> As stated in the previous patch, the Uninorth PCI bridge requires
>> different
>> layouts in its PCI config space accessors.
>>
>> This patch introduces a conversion function that makes it compatible with
>> the way Linux accesses it.
>>
>> I also kept an OpenBIOS compatibility hack in. I think it'd be better to
>> take small steps here and do the config space access rework in OpenBIOS
>> later on. When that's done we can remove that hack.
>>
>> Signed-off-by: Alexander Graf
>> ---
>> hw/unin_pci.c | 35 +++
>> 1 files changed, 35 insertions(+), 0 deletions(-)
>>
>> diff --git a/hw/unin_pci.c b/hw/unin_pci.c
>> index fdb9401..1c49008 100644
>> --- a/hw/unin_pci.c
>> +++ b/hw/unin_pci.c
>> @@ -75,6 +75,40 @@ static void pci_unin_reset(void *opaque)
>> {
>> }
>>
>> +static uint32_t unin_get_config_reg(PCIHostState *s, uint32_t addr)
>> +{
>> +uint32_t retval;
>> +uint32_t reg = s->config_reg;
>> +
>> +if (reg & (1u << 31)) {
>> +/* XXX OpenBIOS compatibility hack */
>> +retval = reg;
>> +addr |= reg & 7;
>> +} else if (reg & 1) {
>> +/* Set upper valid bit and remove lower one */
>> +retval = (reg & ~3u) | (1u << 31);
>> +} else {
>> +uint32_t slot, func;
>> +uint32_t devfn;
>> +
>> +/* Grab CFA0 style values */
>> +slot = ffs(reg & 0xf800) - 1;
>> +func = (reg >> 8) & 7;
>> +devfn = PCI_DEVFN(slot, func);
>> +
>> +/* ... and then convert them to x86 format */
>> +retval = (reg & 0xfc) | (devfn << 8) | (1u << 31);
>
> Is it a good idea to have a helper that encodes reg/dev/fn into a 32 bit
> number? This way this encoding can be changed down the road.
I don't think I understand this comment? :-)
>>>
>>> This puts reg+dev+fn in a format that pci_host can the understand
>>> correct? So it would make sense to have an inline function
>>> in pci host that gets 3 parameters and does the encoding.
>>
>> We're doing the reverse here. We get a uint32_t (host->config_reg) and need
>> to do something with it.
>>
>> We could either call a helper that splits it into bus,dev,fn or we could
>> just put all of them into a single uint32_t again that later on gets
>> interpreted in a specified format.
>>
>> I figured I'd have to touch less code and keep things more stable for the
>> other (non-uninorth) buses if I keep the x86 format as "default format" and
>> just convert to it. Passing a uint32_t is also easier than passing 3 ints
>> :-).
>>
>> Alex
>
> So what the comment above suggests, is adding helper routine
> that gets register device, function and creates 32 bit value
> in "default format".
Oh, so you mean that instead of returning a uint32_t that magically gets
converted inside the conversion function, we'd create another function like
this:
uint32_t pci_host_config_address(int bus, int dev, int fn)
{
return (1u << 31) | (bus << 11) | (dev << 3) | fn;
}
which then would be called at the end of the conversion function?
Alex
[Qemu-devel] Re: [PATCH 2/6] Add config space conversion function for uni_north
On Sun, Jan 03, 2010 at 05:13:12PM +0100, Alexander Graf wrote:
>
> On 03.01.2010, at 16:47, Michael S. Tsirkin wrote:
>
> > On Sun, Jan 03, 2010 at 04:40:12PM +0100, Alexander Graf wrote:
> >>
> >> On 03.01.2010, at 16:32, Michael S. Tsirkin wrote:
> >>
> >>> On Sun, Jan 03, 2010 at 02:50:46AM +0100, Alexander Graf wrote:
> As stated in the previous patch, the Uninorth PCI bridge requires
> different
> layouts in its PCI config space accessors.
>
> This patch introduces a conversion function that makes it compatible with
> the way Linux accesses it.
>
> I also kept an OpenBIOS compatibility hack in. I think it'd be better to
> take small steps here and do the config space access rework in OpenBIOS
> later on. When that's done we can remove that hack.
>
> Signed-off-by: Alexander Graf
> ---
> hw/unin_pci.c | 35 +++
> 1 files changed, 35 insertions(+), 0 deletions(-)
>
> diff --git a/hw/unin_pci.c b/hw/unin_pci.c
> index fdb9401..1c49008 100644
> --- a/hw/unin_pci.c
> +++ b/hw/unin_pci.c
> @@ -75,6 +75,40 @@ static void pci_unin_reset(void *opaque)
> {
> }
>
> +static uint32_t unin_get_config_reg(PCIHostState *s, uint32_t addr)
> +{
> +uint32_t retval;
> +uint32_t reg = s->config_reg;
> +
> +if (reg & (1u << 31)) {
> +/* XXX OpenBIOS compatibility hack */
> +retval = reg;
> +addr |= reg & 7;
> +} else if (reg & 1) {
> +/* Set upper valid bit and remove lower one */
> +retval = (reg & ~3u) | (1u << 31);
> +} else {
> +uint32_t slot, func;
> +uint32_t devfn;
> +
> +/* Grab CFA0 style values */
> +slot = ffs(reg & 0xf800) - 1;
> +func = (reg >> 8) & 7;
> +devfn = PCI_DEVFN(slot, func);
> +
> +/* ... and then convert them to x86 format */
> +retval = (reg & 0xfc) | (devfn << 8) | (1u << 31);
> >>>
> >>> Is it a good idea to have a helper that encodes reg/dev/fn into a 32 bit
> >>> number? This way this encoding can be changed down the road.
> >>
> >> I don't think I understand this comment? :-)
> >
> > This puts reg+dev+fn in a format that pci_host can the understand
> > correct? So it would make sense to have an inline function
> > in pci host that gets 3 parameters and does the encoding.
>
> We're doing the reverse here. We get a uint32_t (host->config_reg) and need
> to do something with it.
>
> We could either call a helper that splits it into bus,dev,fn or we could just
> put all of them into a single uint32_t again that later on gets interpreted
> in a specified format.
>
> I figured I'd have to touch less code and keep things more stable for the
> other (non-uninorth) buses if I keep the x86 format as "default format" and
> just convert to it. Passing a uint32_t is also easier than passing 3 ints :-).
>
> Alex
So what the comment above suggests, is adding helper routine
that gets register device, function and creates 32 bit value
in "default format".
--
MST
[Qemu-devel] Re: [PATCH 2/6] Add config space conversion function for uni_north
On 03.01.2010, at 16:47, Michael S. Tsirkin wrote:
> On Sun, Jan 03, 2010 at 04:40:12PM +0100, Alexander Graf wrote:
>>
>> On 03.01.2010, at 16:32, Michael S. Tsirkin wrote:
>>
>>> On Sun, Jan 03, 2010 at 02:50:46AM +0100, Alexander Graf wrote:
As stated in the previous patch, the Uninorth PCI bridge requires different
layouts in its PCI config space accessors.
This patch introduces a conversion function that makes it compatible with
the way Linux accesses it.
I also kept an OpenBIOS compatibility hack in. I think it'd be better to
take small steps here and do the config space access rework in OpenBIOS
later on. When that's done we can remove that hack.
Signed-off-by: Alexander Graf
---
hw/unin_pci.c | 35 +++
1 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/hw/unin_pci.c b/hw/unin_pci.c
index fdb9401..1c49008 100644
--- a/hw/unin_pci.c
+++ b/hw/unin_pci.c
@@ -75,6 +75,40 @@ static void pci_unin_reset(void *opaque)
{
}
+static uint32_t unin_get_config_reg(PCIHostState *s, uint32_t addr)
+{
+uint32_t retval;
+uint32_t reg = s->config_reg;
+
+if (reg & (1u << 31)) {
+/* XXX OpenBIOS compatibility hack */
+retval = reg;
+addr |= reg & 7;
+} else if (reg & 1) {
+/* Set upper valid bit and remove lower one */
+retval = (reg & ~3u) | (1u << 31);
+} else {
+uint32_t slot, func;
+uint32_t devfn;
+
+/* Grab CFA0 style values */
+slot = ffs(reg & 0xf800) - 1;
+func = (reg >> 8) & 7;
+devfn = PCI_DEVFN(slot, func);
+
+/* ... and then convert them to x86 format */
+retval = (reg & 0xfc) | (devfn << 8) | (1u << 31);
>>>
>>> Is it a good idea to have a helper that encodes reg/dev/fn into a 32 bit
>>> number? This way this encoding can be changed down the road.
>>
>> I don't think I understand this comment? :-)
>
> This puts reg+dev+fn in a format that pci_host can the understand
> correct? So it would make sense to have an inline function
> in pci host that gets 3 parameters and does the encoding.
We're doing the reverse here. We get a uint32_t (host->config_reg) and need to
do something with it. We could either call a helper that splits it into
bus,dev,fn or we could just put all of them into a single uint32_t again that
later on gets interpreted in a specified format.
I figured I'd have to touch less code and keep things more stable for the other
(non-uninorth) buses if I keep the x86 format as "default format" and just
convert to it. Passing a uint32_t is also easier than passing 3 ints :-).
Alex
[Qemu-devel] Re: [PATCH 1/6] Make config space accessor host bus trapable
On 03.01.2010, at 16:45, Michael S. Tsirkin wrote:
> On Sun, Jan 03, 2010 at 02:50:45AM +0100, Alexander Graf wrote:
>> Different host buses may have different layouts for config space accessors.
>>
>> The Mac U3 for example uses the following define to access Type 0 (directly
>> attached) devices:
>>
>> #define MACRISC_CFA0(devfn, off)\
>>((1 << (unsigned int)PCI_SLOT(dev_fn)) \
>>| (((unsigned int)PCI_FUNC(dev_fn)) << 8) \
>>| (((unsigned int)(off)) & 0xFCUL))
>>
>> Obviously, this is different from the encoding we interpret in qemu. In
>> order to let host controllers take some influence there, we can just
>> introduce a converter function that takes whatever accessor we have and
>> makes a qemu understandable one out of it.
>>
>> This patch is the groundwork for Uninorth PCI config space fixes.
>>
>> Signed-off-by: Alexander Graf
>> CC: Michael S. Tsirkin
>
> Thanks!
>
> It always bothered me that the code in pci_host uses x86 encoding and
> other architectures are converted to x86. As long as we are adding
> redirection, maybe we should get rid of this, and make get_config_reg
> return register and devfn separately, so we don't need to encode/decode
> back and forth?
Hmm, when touching code I'm not 100% sure what I'm doing I usually try to build
on stuff that is there already. That's exactly what happened here.
I'm personally not against defining the x86 format as qemu default. That way
everyone knows what to deal with. I'm not sure if PCI defines anything that
couldn't be represented by the x86 encoding in 32 bits. I actually doubt it. So
it seems like a pretty good fit, especially given that all the other code is
already in place.
> OTOH if we are after a quick fix, won't it be simpler to have
> unin_pci simply use its own routines instead of pci_host_conf_register_mmio
> etc?
Hm, I figured this is less work :-).
>
>> ---
>> hw/apb_pci.c |1 +
>> hw/grackle_pci.c |1 +
>> hw/gt64xxx.c |1 +
>> hw/pci_host.c | 11 +++
>> hw/pci_host.h |5 +
>> hw/pci_host_template.h | 30 ++
>> hw/piix_pci.c |1 +
>> hw/ppc4xx_pci.c|1 +
>> hw/ppce500_pci.c |1 +
>> hw/unin_pci.c |1 +
>> 10 files changed, 41 insertions(+), 12 deletions(-)
>>
>> diff --git a/hw/apb_pci.c b/hw/apb_pci.c
>> index f05308b..fedb84c 100644
>> --- a/hw/apb_pci.c
>> +++ b/hw/apb_pci.c
>> @@ -222,6 +222,7 @@ PCIBus *pci_apb_init(target_phys_addr_t special_base,
>> /* mem_data */
>> sysbus_mmio_map(s, 3, mem_base);
>> d = FROM_SYSBUS(APBState, s);
>> +pci_host_init(&d->host_state);
>> d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
>> pci_apb_set_irq, pci_pbm_map_irq,
>> pic,
>> 0, 32);
>> diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c
>> index ee4fed5..7feba63 100644
>> --- a/hw/grackle_pci.c
>> +++ b/hw/grackle_pci.c
>> @@ -88,6 +88,7 @@ PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic)
>> qdev_init_nofail(dev);
>> s = sysbus_from_qdev(dev);
>> d = FROM_SYSBUS(GrackleState, s);
>> +pci_host_init(&d->host_state);
>> d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
>> pci_grackle_set_irq,
>> pci_grackle_map_irq,
>> diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
>> index fb7f5bd..8cf93ca 100644
>> --- a/hw/gt64xxx.c
>> +++ b/hw/gt64xxx.c
>> @@ -1120,6 +1120,7 @@ PCIBus *pci_gt64120_init(qemu_irq *pic)
>> s = qemu_mallocz(sizeof(GT64120State));
>> s->pci = qemu_mallocz(sizeof(GT64120PCIState));
>>
>> +pci_host_init(s->pci);
>> s->pci->bus = pci_register_bus(NULL, "pci",
>>pci_gt64120_set_irq, pci_gt64120_map_irq,
>>pic, 144, 4);
>> diff --git a/hw/pci_host.c b/hw/pci_host.c
>> index eeb8dee..2d12887 100644
>> --- a/hw/pci_host.c
>> +++ b/hw/pci_host.c
>> @@ -228,3 +228,14 @@ void pci_host_data_register_ioport(pio_addr_t ioport,
>> PCIHostState *s)
>> register_ioport_read(ioport, 4, 2, pci_host_data_readw_ioport, s);
>> register_ioport_read(ioport, 4, 4, pci_host_data_readl_ioport, s);
>> }
>> +
>> +/* This implements the default x86 way of interpreting the config register
>> */
>> +static uint32_t pci_host_get_config_reg(PCIHostState *s, uint32_t addr)
>> +{
>> +return s->config_reg | (addr & 3);
>> +}
>> +
>> +void pci_host_init(PCIHostState *s)
>> +{
>> +s->get_config_reg = pci_host_get_config_reg;
>> +}
>
> So config_reg is always set but only used for PC now?
> This is not very pretty ...
config_reg is used by the template.h helpers. So everyone should use it.
get_config_reg is always set. It's set to pci_host_get_config_reg as default
and can be overridden by a host bus if it knows it's using a different layo
[Qemu-devel] Re: [PATCH 2/6] Add config space conversion function for uni_north
On Sun, Jan 03, 2010 at 04:40:12PM +0100, Alexander Graf wrote:
>
> On 03.01.2010, at 16:32, Michael S. Tsirkin wrote:
>
> > On Sun, Jan 03, 2010 at 02:50:46AM +0100, Alexander Graf wrote:
> >> As stated in the previous patch, the Uninorth PCI bridge requires different
> >> layouts in its PCI config space accessors.
> >>
> >> This patch introduces a conversion function that makes it compatible with
> >> the way Linux accesses it.
> >>
> >> I also kept an OpenBIOS compatibility hack in. I think it'd be better to
> >> take small steps here and do the config space access rework in OpenBIOS
> >> later on. When that's done we can remove that hack.
> >>
> >> Signed-off-by: Alexander Graf
> >> ---
> >> hw/unin_pci.c | 35 +++
> >> 1 files changed, 35 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/hw/unin_pci.c b/hw/unin_pci.c
> >> index fdb9401..1c49008 100644
> >> --- a/hw/unin_pci.c
> >> +++ b/hw/unin_pci.c
> >> @@ -75,6 +75,40 @@ static void pci_unin_reset(void *opaque)
> >> {
> >> }
> >>
> >> +static uint32_t unin_get_config_reg(PCIHostState *s, uint32_t addr)
> >> +{
> >> +uint32_t retval;
> >> +uint32_t reg = s->config_reg;
> >> +
> >> +if (reg & (1u << 31)) {
> >> +/* XXX OpenBIOS compatibility hack */
> >> +retval = reg;
> >> +addr |= reg & 7;
> >> +} else if (reg & 1) {
> >> +/* Set upper valid bit and remove lower one */
> >> +retval = (reg & ~3u) | (1u << 31);
> >> +} else {
> >> +uint32_t slot, func;
> >> +uint32_t devfn;
> >> +
> >> +/* Grab CFA0 style values */
> >> +slot = ffs(reg & 0xf800) - 1;
> >> +func = (reg >> 8) & 7;
> >> +devfn = PCI_DEVFN(slot, func);
> >> +
> >> +/* ... and then convert them to x86 format */
> >> +retval = (reg & 0xfc) | (devfn << 8) | (1u << 31);
> >
> > Is it a good idea to have a helper that encodes reg/dev/fn into a 32 bit
> > number? This way this encoding can be changed down the road.
>
> I don't think I understand this comment? :-)
>
> >
> >> +}
> >> +
> >> +retval &= ~3u;
> >> +retval |= (addr & 7);
> >> +
> >> +UNIN_DPRINTF("Converted config space accessor %08x/%08x -> %08x\n",
> >> + reg, addr, retval);
> >> +
> >> +return retval;
> >> +}
> >> +
> >> static int pci_unin_main_init_device(SysBusDevice *dev)
> >> {
> >> UNINState *s;
> >> @@ -85,6 +119,7 @@ static int pci_unin_main_init_device(SysBusDevice *dev)
> >> s = FROM_SYSBUS(UNINState, dev);
> >>
> >> pci_host_init(&s->host_state);
> >> +s->host_state.get_config_reg = unin_get_config_reg;
> >
> > This looks slightly ugly: let's make pci_host_init accept
> > the conversion function instead?
>
> Hmm. My guess was that 99% of the host adapters don't need a replacement
> function, so I wanted to keep the defaults simple. If we later on add
> additional helpers, that would also be easier, as we wouldn't need to touch
> every initializer call but only the overriding ones.
>
OK.
> Alex
[Qemu-devel] Re: [PATCH 2/6] Add config space conversion function for uni_north
On Sun, Jan 03, 2010 at 04:40:12PM +0100, Alexander Graf wrote:
>
> On 03.01.2010, at 16:32, Michael S. Tsirkin wrote:
>
> > On Sun, Jan 03, 2010 at 02:50:46AM +0100, Alexander Graf wrote:
> >> As stated in the previous patch, the Uninorth PCI bridge requires different
> >> layouts in its PCI config space accessors.
> >>
> >> This patch introduces a conversion function that makes it compatible with
> >> the way Linux accesses it.
> >>
> >> I also kept an OpenBIOS compatibility hack in. I think it'd be better to
> >> take small steps here and do the config space access rework in OpenBIOS
> >> later on. When that's done we can remove that hack.
> >>
> >> Signed-off-by: Alexander Graf
> >> ---
> >> hw/unin_pci.c | 35 +++
> >> 1 files changed, 35 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/hw/unin_pci.c b/hw/unin_pci.c
> >> index fdb9401..1c49008 100644
> >> --- a/hw/unin_pci.c
> >> +++ b/hw/unin_pci.c
> >> @@ -75,6 +75,40 @@ static void pci_unin_reset(void *opaque)
> >> {
> >> }
> >>
> >> +static uint32_t unin_get_config_reg(PCIHostState *s, uint32_t addr)
> >> +{
> >> +uint32_t retval;
> >> +uint32_t reg = s->config_reg;
> >> +
> >> +if (reg & (1u << 31)) {
> >> +/* XXX OpenBIOS compatibility hack */
> >> +retval = reg;
> >> +addr |= reg & 7;
> >> +} else if (reg & 1) {
> >> +/* Set upper valid bit and remove lower one */
> >> +retval = (reg & ~3u) | (1u << 31);
> >> +} else {
> >> +uint32_t slot, func;
> >> +uint32_t devfn;
> >> +
> >> +/* Grab CFA0 style values */
> >> +slot = ffs(reg & 0xf800) - 1;
> >> +func = (reg >> 8) & 7;
> >> +devfn = PCI_DEVFN(slot, func);
> >> +
> >> +/* ... and then convert them to x86 format */
> >> +retval = (reg & 0xfc) | (devfn << 8) | (1u << 31);
> >
> > Is it a good idea to have a helper that encodes reg/dev/fn into a 32 bit
> > number? This way this encoding can be changed down the road.
>
> I don't think I understand this comment? :-)
This puts reg+dev+fn in a format that pci_host can the understand
correct? So it would make sense to have an inline function
in pci host that gets 3 parameters and does the encoding.
> >
> >> +}
> >> +
> >> +retval &= ~3u;
> >> +retval |= (addr & 7);
> >> +
> >> +UNIN_DPRINTF("Converted config space accessor %08x/%08x -> %08x\n",
> >> + reg, addr, retval);
> >> +
> >> +return retval;
> >> +}
> >> +
> >> static int pci_unin_main_init_device(SysBusDevice *dev)
> >> {
> >> UNINState *s;
> >> @@ -85,6 +119,7 @@ static int pci_unin_main_init_device(SysBusDevice *dev)
> >> s = FROM_SYSBUS(UNINState, dev);
> >>
> >> pci_host_init(&s->host_state);
> >> +s->host_state.get_config_reg = unin_get_config_reg;
> >
> > This looks slightly ugly: let's make pci_host_init accept
> > the conversion function instead?
>
> Hmm. My guess was that 99% of the host adapters don't need a replacement
> function, so I wanted to keep the defaults simple. If we later on add
> additional helpers, that would also be easier, as we wouldn't need to touch
> every initializer call but only the overriding ones.
>
>
> Alex
[Qemu-devel] Re: [PATCH 1/6] Make config space accessor host bus trapable
On Sun, Jan 03, 2010 at 02:50:45AM +0100, Alexander Graf wrote:
> Different host buses may have different layouts for config space accessors.
>
> The Mac U3 for example uses the following define to access Type 0 (directly
> attached) devices:
>
> #define MACRISC_CFA0(devfn, off)\
> ((1 << (unsigned int)PCI_SLOT(dev_fn)) \
> | (((unsigned int)PCI_FUNC(dev_fn)) << 8) \
> | (((unsigned int)(off)) & 0xFCUL))
>
> Obviously, this is different from the encoding we interpret in qemu. In
> order to let host controllers take some influence there, we can just
> introduce a converter function that takes whatever accessor we have and
> makes a qemu understandable one out of it.
>
> This patch is the groundwork for Uninorth PCI config space fixes.
>
> Signed-off-by: Alexander Graf
> CC: Michael S. Tsirkin
Thanks!
It always bothered me that the code in pci_host uses x86 encoding and
other architectures are converted to x86. As long as we are adding
redirection, maybe we should get rid of this, and make get_config_reg
return register and devfn separately, so we don't need to encode/decode
back and forth?
OTOH if we are after a quick fix, won't it be simpler to have
unin_pci simply use its own routines instead of pci_host_conf_register_mmio
etc?
> ---
> hw/apb_pci.c |1 +
> hw/grackle_pci.c |1 +
> hw/gt64xxx.c |1 +
> hw/pci_host.c | 11 +++
> hw/pci_host.h |5 +
> hw/pci_host_template.h | 30 ++
> hw/piix_pci.c |1 +
> hw/ppc4xx_pci.c|1 +
> hw/ppce500_pci.c |1 +
> hw/unin_pci.c |1 +
> 10 files changed, 41 insertions(+), 12 deletions(-)
>
> diff --git a/hw/apb_pci.c b/hw/apb_pci.c
> index f05308b..fedb84c 100644
> --- a/hw/apb_pci.c
> +++ b/hw/apb_pci.c
> @@ -222,6 +222,7 @@ PCIBus *pci_apb_init(target_phys_addr_t special_base,
> /* mem_data */
> sysbus_mmio_map(s, 3, mem_base);
> d = FROM_SYSBUS(APBState, s);
> +pci_host_init(&d->host_state);
> d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
> pci_apb_set_irq, pci_pbm_map_irq,
> pic,
> 0, 32);
> diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c
> index ee4fed5..7feba63 100644
> --- a/hw/grackle_pci.c
> +++ b/hw/grackle_pci.c
> @@ -88,6 +88,7 @@ PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic)
> qdev_init_nofail(dev);
> s = sysbus_from_qdev(dev);
> d = FROM_SYSBUS(GrackleState, s);
> +pci_host_init(&d->host_state);
> d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
> pci_grackle_set_irq,
> pci_grackle_map_irq,
> diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
> index fb7f5bd..8cf93ca 100644
> --- a/hw/gt64xxx.c
> +++ b/hw/gt64xxx.c
> @@ -1120,6 +1120,7 @@ PCIBus *pci_gt64120_init(qemu_irq *pic)
> s = qemu_mallocz(sizeof(GT64120State));
> s->pci = qemu_mallocz(sizeof(GT64120PCIState));
>
> +pci_host_init(s->pci);
> s->pci->bus = pci_register_bus(NULL, "pci",
> pci_gt64120_set_irq, pci_gt64120_map_irq,
> pic, 144, 4);
> diff --git a/hw/pci_host.c b/hw/pci_host.c
> index eeb8dee..2d12887 100644
> --- a/hw/pci_host.c
> +++ b/hw/pci_host.c
> @@ -228,3 +228,14 @@ void pci_host_data_register_ioport(pio_addr_t ioport,
> PCIHostState *s)
> register_ioport_read(ioport, 4, 2, pci_host_data_readw_ioport, s);
> register_ioport_read(ioport, 4, 4, pci_host_data_readl_ioport, s);
> }
> +
> +/* This implements the default x86 way of interpreting the config register */
> +static uint32_t pci_host_get_config_reg(PCIHostState *s, uint32_t addr)
> +{
> +return s->config_reg | (addr & 3);
> +}
> +
> +void pci_host_init(PCIHostState *s)
> +{
> +s->get_config_reg = pci_host_get_config_reg;
> +}
So config_reg is always set but only used for PC now?
This is not very pretty ...
> diff --git a/hw/pci_host.h b/hw/pci_host.h
> index a006687..90a4c63 100644
> --- a/hw/pci_host.h
> +++ b/hw/pci_host.h
> @@ -30,14 +30,19 @@
>
> #include "sysbus.h"
>
> +/* for config space access */
> +typedef uint32_t (*pci_config_reg_fn)(PCIHostState *s, uint32_t addr);
> +
> struct PCIHostState {
> SysBusDevice busdev;
> +pci_config_reg_fn get_config_reg;
> uint32_t config_reg;
> PCIBus *bus;
> };
>
> void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len);
> uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len);
> +void pci_host_init(PCIHostState *s);
>
> /* for mmio */
> int pci_host_conf_register_mmio(PCIHostState *s);
> diff --git a/hw/pci_host_template.h b/hw/pci_host_template.h
> index 11e6c88..55aa85e 100644
> --- a/hw/pci_host_template.h
> +++ b/hw/pci_host_template.h
> @@ -29,48 +29,52 @@ static void glue(pci_hos
[Qemu-devel] Re: [PATCH 2/6] Add config space conversion function for uni_north
On 03.01.2010, at 16:32, Michael S. Tsirkin wrote:
> On Sun, Jan 03, 2010 at 02:50:46AM +0100, Alexander Graf wrote:
>> As stated in the previous patch, the Uninorth PCI bridge requires different
>> layouts in its PCI config space accessors.
>>
>> This patch introduces a conversion function that makes it compatible with
>> the way Linux accesses it.
>>
>> I also kept an OpenBIOS compatibility hack in. I think it'd be better to
>> take small steps here and do the config space access rework in OpenBIOS
>> later on. When that's done we can remove that hack.
>>
>> Signed-off-by: Alexander Graf
>> ---
>> hw/unin_pci.c | 35 +++
>> 1 files changed, 35 insertions(+), 0 deletions(-)
>>
>> diff --git a/hw/unin_pci.c b/hw/unin_pci.c
>> index fdb9401..1c49008 100644
>> --- a/hw/unin_pci.c
>> +++ b/hw/unin_pci.c
>> @@ -75,6 +75,40 @@ static void pci_unin_reset(void *opaque)
>> {
>> }
>>
>> +static uint32_t unin_get_config_reg(PCIHostState *s, uint32_t addr)
>> +{
>> +uint32_t retval;
>> +uint32_t reg = s->config_reg;
>> +
>> +if (reg & (1u << 31)) {
>> +/* XXX OpenBIOS compatibility hack */
>> +retval = reg;
>> +addr |= reg & 7;
>> +} else if (reg & 1) {
>> +/* Set upper valid bit and remove lower one */
>> +retval = (reg & ~3u) | (1u << 31);
>> +} else {
>> +uint32_t slot, func;
>> +uint32_t devfn;
>> +
>> +/* Grab CFA0 style values */
>> +slot = ffs(reg & 0xf800) - 1;
>> +func = (reg >> 8) & 7;
>> +devfn = PCI_DEVFN(slot, func);
>> +
>> +/* ... and then convert them to x86 format */
>> +retval = (reg & 0xfc) | (devfn << 8) | (1u << 31);
>
> Is it a good idea to have a helper that encodes reg/dev/fn into a 32 bit
> number? This way this encoding can be changed down the road.
I don't think I understand this comment? :-)
>
>> +}
>> +
>> +retval &= ~3u;
>> +retval |= (addr & 7);
>> +
>> +UNIN_DPRINTF("Converted config space accessor %08x/%08x -> %08x\n",
>> + reg, addr, retval);
>> +
>> +return retval;
>> +}
>> +
>> static int pci_unin_main_init_device(SysBusDevice *dev)
>> {
>> UNINState *s;
>> @@ -85,6 +119,7 @@ static int pci_unin_main_init_device(SysBusDevice *dev)
>> s = FROM_SYSBUS(UNINState, dev);
>>
>> pci_host_init(&s->host_state);
>> +s->host_state.get_config_reg = unin_get_config_reg;
>
> This looks slightly ugly: let's make pci_host_init accept
> the conversion function instead?
Hmm. My guess was that 99% of the host adapters don't need a replacement
function, so I wanted to keep the defaults simple. If we later on add
additional helpers, that would also be easier, as we wouldn't need to touch
every initializer call but only the overriding ones.
Alex
[Qemu-devel] Re: [PATCH 2/6] Add config space conversion function for uni_north
On Sun, Jan 03, 2010 at 02:50:46AM +0100, Alexander Graf wrote:
> As stated in the previous patch, the Uninorth PCI bridge requires different
> layouts in its PCI config space accessors.
>
> This patch introduces a conversion function that makes it compatible with
> the way Linux accesses it.
>
> I also kept an OpenBIOS compatibility hack in. I think it'd be better to
> take small steps here and do the config space access rework in OpenBIOS
> later on. When that's done we can remove that hack.
>
> Signed-off-by: Alexander Graf
> ---
> hw/unin_pci.c | 35 +++
> 1 files changed, 35 insertions(+), 0 deletions(-)
>
> diff --git a/hw/unin_pci.c b/hw/unin_pci.c
> index fdb9401..1c49008 100644
> --- a/hw/unin_pci.c
> +++ b/hw/unin_pci.c
> @@ -75,6 +75,40 @@ static void pci_unin_reset(void *opaque)
> {
> }
>
> +static uint32_t unin_get_config_reg(PCIHostState *s, uint32_t addr)
> +{
> +uint32_t retval;
> +uint32_t reg = s->config_reg;
> +
> +if (reg & (1u << 31)) {
> +/* XXX OpenBIOS compatibility hack */
> +retval = reg;
> +addr |= reg & 7;
> +} else if (reg & 1) {
> +/* Set upper valid bit and remove lower one */
> +retval = (reg & ~3u) | (1u << 31);
> +} else {
> +uint32_t slot, func;
> +uint32_t devfn;
> +
> +/* Grab CFA0 style values */
> +slot = ffs(reg & 0xf800) - 1;
> +func = (reg >> 8) & 7;
> +devfn = PCI_DEVFN(slot, func);
> +
> +/* ... and then convert them to x86 format */
> +retval = (reg & 0xfc) | (devfn << 8) | (1u << 31);
Is it a good idea to have a helper that encodes reg/dev/fn into a 32 bit
number? This way this encoding can be changed down the road.
> +}
> +
> +retval &= ~3u;
> +retval |= (addr & 7);
> +
> +UNIN_DPRINTF("Converted config space accessor %08x/%08x -> %08x\n",
> + reg, addr, retval);
> +
> +return retval;
> +}
> +
> static int pci_unin_main_init_device(SysBusDevice *dev)
> {
> UNINState *s;
> @@ -85,6 +119,7 @@ static int pci_unin_main_init_device(SysBusDevice *dev)
> s = FROM_SYSBUS(UNINState, dev);
>
> pci_host_init(&s->host_state);
> +s->host_state.get_config_reg = unin_get_config_reg;
This looks slightly ugly: let's make pci_host_init accept
the conversion function instead?
> pci_mem_config = pci_host_conf_register_mmio(&s->host_state);
> pci_mem_data = pci_host_data_register_mmio(&s->host_state);
> sysbus_init_mmio(dev, 0x1000, pci_mem_config);
> --
> 1.6.0.2
>
>
[Qemu-devel] Re: [PATCH v2] Drop --whole-archive and static libraries
On Thu, 31 Dec 2009, Andreas F?rber wrote:
v2:
- Don't try to include /config.mak for user emulators
- Changes to user object paths ("Quickfix for libuser.a drop") were obsoleted
by "user_only: compile everything with -fpie" (Kirill A. Shutemov)
Hi,
I have succesfully used the v2 patch on my OpenSolaris/SPARC host and it
seems to fix the Solaris linking issue.
/Palle
Re: [Qemu-devel] [PATCH v2] Drop --whole-archive and static libraries
2009/12/31 Andreas Färber :
> From: Andreas Färber
>
> Juan has contributed a cool Makefile infrastructure that enables us to drop
> static libraries completely:
>
> Move shared obj-y definitions to Makefile.objs, prefixed {common-,hw-,user-},
> and link those object files directly into the executables.
>
> Replace HWLIB by HWDIR, specifying only the directory.
>
> Drop --whole-archive and ARLIBS in Makefiles and configure.
>
> Drop GENERATED_HEADERS dependency in rules.mak, since this rebuilds all
> common objects after generating a target-specific header; add dependency
> rules to Makefile and Makefile.target instead.
>
> v2:
> - Don't try to include /config.mak for user emulators
> - Changes to user object paths ("Quickfix for libuser.a drop") were obsoleted
> by "user_only: compile everything with -fpie" (Kirill A. Shutemov)
Breaks build:
CCi386-softmmu/i386-dis.o
make[1]: *** No rule to make target `/loader.o', needed by `qemu'. Stop.
Re: [Qemu-devel] [PATCH] pass env to raise_exception if called outside of op_helper code
Thanks, applied.
On Sun, Jan 3, 2010 at 12:09 PM, Igor V. Kovalenko
wrote:
> From: Igor V. Kovalenko
>
> - this fixes stepping with gdb, where do_unassigned_access
> may be called from gdb handler, outside of generated code
>
> Signed-off-by: Igor V. Kovalenko
> ---
> target-sparc/op_helper.c | 7 +--
> 1 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
> index 4e0a0e3..bd01a5e 100644
> --- a/target-sparc/op_helper.c
> +++ b/target-sparc/op_helper.c
> @@ -3686,21 +3686,24 @@ void do_unassigned_access(target_phys_addr_t addr,
> int is_write, int is_exec,
> void do_unassigned_access(target_phys_addr_t addr, int is_write, int is_exec,
> int is_asi, int size)
> {
> -#ifdef DEBUG_UNASSIGNED
> CPUState *saved_env;
>
> /* XXX: hack to restore env in all cases, even if not called from
> generated code */
> saved_env = env;
> env = cpu_single_env;
> +
> +#ifdef DEBUG_UNASSIGNED
> printf("Unassigned mem access to " TARGET_FMT_plx " from " TARGET_FMT_lx
> "\n", addr, env->pc);
> - env = saved_env;
> #endif
> +
> if (is_exec)
> raise_exception(TT_CODE_ACCESS);
> else
> raise_exception(TT_DATA_ACCESS);
> +
> + env = saved_env;
> }
> #endif
>
>
>
>
>
Re: [Qemu-devel] [PATCH] sparc64: switch to MMU global registers in more MMU related traps
Thanks, applied. On Sun, Jan 3, 2010 at 12:01 PM, Igor V. Kovalenko wrote: > From: Igor V. Kovalenko > > - extended range of MMU related traps which use MMU global registers, > as listed in Ultrasparc-IIi document > - no visible changes, since emulation do not cause added traps > > Signed-off-by: Igor V. Kovalenko > --- > target-sparc/op_helper.c | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c > index c3cc0a4..362a557 100644 > --- a/target-sparc/op_helper.c > +++ b/target-sparc/op_helper.c > @@ -3447,10 +3447,10 @@ void do_interrupt(CPUState *env) > change_pstate(PS_PEF | PS_PRIV | PS_IG); > break; > case TT_TFAULT: > - case TT_TMISS: > case TT_DFAULT: > - case TT_DMISS: > - case TT_DPROT: > + case TT_TMISS ... TT_TMISS+3: > + case TT_DMISS ... TT_DMISS+3: > + case TT_DPROT ... TT_DPROT+3: > change_pstate(PS_PEF | PS_PRIV | PS_MG); > break; > default: > > > >
[Qemu-devel] [PATCH] pass env to raise_exception if called outside of op_helper code
From: Igor V. Kovalenko
- this fixes stepping with gdb, where do_unassigned_access
may be called from gdb handler, outside of generated code
Signed-off-by: Igor V. Kovalenko
---
target-sparc/op_helper.c |7 +--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
index 4e0a0e3..bd01a5e 100644
--- a/target-sparc/op_helper.c
+++ b/target-sparc/op_helper.c
@@ -3686,21 +3686,24 @@ void do_unassigned_access(target_phys_addr_t addr, int
is_write, int is_exec,
void do_unassigned_access(target_phys_addr_t addr, int is_write, int is_exec,
int is_asi, int size)
{
-#ifdef DEBUG_UNASSIGNED
CPUState *saved_env;
/* XXX: hack to restore env in all cases, even if not called from
generated code */
saved_env = env;
env = cpu_single_env;
+
+#ifdef DEBUG_UNASSIGNED
printf("Unassigned mem access to " TARGET_FMT_plx " from " TARGET_FMT_lx
"\n", addr, env->pc);
-env = saved_env;
#endif
+
if (is_exec)
raise_exception(TT_CODE_ACCESS);
else
raise_exception(TT_DATA_ACCESS);
+
+env = saved_env;
}
#endif
[Qemu-devel] [PATCH] sparc64: switch to MMU global registers in more MMU related traps
From: Igor V. Kovalenko - extended range of MMU related traps which use MMU global registers, as listed in Ultrasparc-IIi document - no visible changes, since emulation do not cause added traps Signed-off-by: Igor V. Kovalenko --- target-sparc/op_helper.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c index c3cc0a4..362a557 100644 --- a/target-sparc/op_helper.c +++ b/target-sparc/op_helper.c @@ -3447,10 +3447,10 @@ void do_interrupt(CPUState *env) change_pstate(PS_PEF | PS_PRIV | PS_IG); break; case TT_TFAULT: -case TT_TMISS: case TT_DFAULT: -case TT_DMISS: -case TT_DPROT: +case TT_TMISS ... TT_TMISS+3: +case TT_DMISS ... TT_DMISS+3: +case TT_DPROT ... TT_DPROT+3: change_pstate(PS_PEF | PS_PRIV | PS_MG); break; default:
Re: [Qemu-devel] [PATCH] PPC: Add wrapper for target long DCR operations
Alexander Graf schrieb: > The recent transition to always have the DCR helper functions take 32 bit > values broke the PPC64 target, as tlong became 64 bits there. > > This patch moves all translate.c callers to a _tl function that simply > calls the uint32_t functions. That way we don't need to mess with TCG > trying to pass registers as uint32_t variables to functions. > > Fixes PPC64 build with --enable-debug-tcg > > Signed-off-by: Alexander Graf > Reported-by: Stefan Weil Acked-by: Stefan Weil Happy new year, Stefan
