On 04.11.2009, at 07:14, Isaku Yamahata wrote:
On Tue, Nov 03, 2009 at 03:45:12PM +0200, Michael S. Tsirkin wrote:
--- a/hw/pci_host.c
+++ b/hw/pci_host.c
@@ -32,6 +32,114 @@ do { printf("pci_host_data: " fmt , ##
__VA_ARGS__); } while (0)
#define PCI_DPRINTF(fmt, ...)
#endif
+static void pci_host_config_writel(void *opaque,
target_phys_addr_t addr,
+ uint32_t val)
+{
+ PCIHostState *s = opaque;
+
+#ifdef TARGET_WORDS_BIGENDIAN
+ val = bswap32(val);
+#endif
I know you just copied it, but isn't this just
val = le32_to_cpu(val);
?
Makes sense.
+static void pci_host_config_writel_noswap(void *opaque,
+ target_phys_addr_t addr,
+ uint32_t val)
+{
+ PCIHostState *s = opaque;
+
+ PCI_DPRINTF("%s addr " TARGET_FMT_plx " val %"PRIx32"\n",
+ __func__, addr, val);
+ s->config_reg = val;
+}
+
+static uint32_t pci_host_config_readl_noswap(void *opaque,
+ target_phys_addr_t
addr)
+{
+ PCIHostState *s = opaque;
+ uint32_t val = s->config_reg;
+
+ PCI_DPRINTF("%s addr " TARGET_FMT_plx " val %"PRIx32"\n",
+ __func__, addr, val);
+ return val;
+}
+
+static CPUWriteMemoryFunc * const pci_host_config_write_noswap[]
= {
+ &pci_host_config_writel_noswap,
+ &pci_host_config_writel_noswap,
+ &pci_host_config_writel_noswap,
+};
+
+static CPUReadMemoryFunc * const pci_host_config_read_noswap[] = {
+ &pci_host_config_readl_noswap,
+ &pci_host_config_readl_noswap,
+ &pci_host_config_readl_noswap,
+};
+
+int pci_host_config_register_io_memory_noswap(PCIHostState *s)
This name is clearly too long,
as a result when you use it you run over the 80 character
limit. Let's not fix it, let's make name shorter.
io_memory -> memory?
+{
+ return cpu_register_io_memory(pci_host_config_read_noswap,
+ pci_host_config_write_noswap, s);
+}
Do you happen to know whether no swap is really needed? I suspect
some
of these places really only happen to work because everyone uses
intel,
so they simply would not work on ppc host.
I just followed the original code not to break it.
I dug into the commit logs a bit.
Liu, Aurelian and Alexander, could you give me a comment on
whether those functions needs byte swap (le32_to_cpu()) or not.
- ppcie500_cfgaddr_{write, read}l() in ppce500_pci.c
- pci_unin_config_{write, read}l() in unin_pci.c
ppce500_pci.c case:
The related commit is 74c62ba88902575be9ac3badf95d773470884b1c.
The comment on the top in the file says that it is derived from
ppc4xx_pci.c.
ppc4xx_pci.c has byte swap, on the other hand ppce500_pci.c doesn't.
So I'm inclined to suspect that the byte swap was dropped accidently.
However I don't know its pci host bridge spec.
unin_pci.c case:
I don't know its spec neither.
The following changeset seems to remove the byte swap deliberately.
Alexander, could you please give us comment on it?
Phew - I frankly don't remember. Something broke because some bits
were strangely mangled and the way it's not it worked more than it did
before :-). The whole Uninorth thing is still utterly broken, so
please don't take anything from there as example.
I do know that Hollis was working a lot about LE/BE issues on PPC in
Qemu, so he's probably the best person to CC and ask here.
Alex
