Re: [Qemu-devel] [PATCH master/stable-1.0] pci: fix corrupted pci conf index register by unaligned write

2012-04-15 Thread Michael S. Tsirkin 
On Sat, Apr 14, 2012 at 07:15:31PM +0400, Michael Tokarev wrote:
 Ping?
 
 Its been 4 months ago, and NetBSD still can't be booted in qemu...
 
 I understand this can be done differently, but the patch in
 question changed behavour and it caused a visible regression,
 so let's fix this regression by restoring previous working
 behavour and start thinking how it should be made better
 after that... okay?  ;)
 
 Thank you!

I agree.
Applied, thanks to everyone.

Re: [Qemu-devel] [PATCH master/stable-1.0] pci: fix corrupted pci conf index register by unaligned write

2012-04-15 Thread Avi Kivity 
On 04/15/2012 12:11 PM, Michael S. Tsirkin wrote:
 On Sat, Apr 14, 2012 at 07:15:31PM +0400, Michael Tokarev wrote:
  Ping?
  
  Its been 4 months ago, and NetBSD still can't be booted in qemu...
  
  I understand this can be done differently, but the patch in
  question changed behavour and it caused a visible regression,
  so let's fix this regression by restoring previous working
  behavour and start thinking how it should be made better
  after that... okay?  ;)
  
  Thank you!

 I agree.
 Applied, thanks to everyone.

Ok, not sending my own pull request.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH master/stable-1.0] pci: fix corrupted pci conf index register by unaligned write

2012-04-14 Thread Michael Tokarev 
Ping?

Its been 4 months ago, and NetBSD still can't be booted in qemu...

I understand this can be done differently, but the patch in
question changed behavour and it caused a visible regression,
so let's fix this regression by restoring previous working
behavour and start thinking how it should be made better
after that... okay?  ;)

Thank you!

On 04.01.2012 18:28, Avi Kivity wrote:
 Commit d0ed8076cbdc261 converted the PCI config access to the memory
 API, but also inadvertantly changed it to accept unaligned writes,
 and corrupt the index register in the process.  This causes a regression
 booting NetBSD.
 
 Fix by ignoring unaligned or non-dword writes.
 
 https://bugs.launchpad.net/qemu/+bug/897771
 
 Reported-by: Andreas Gustafsson g...@gson.org
 Signed-off-by: Avi Kivity a...@redhat.com
 ---
 
  hw/pci_host.c |3 +++
  1 files changed, 3 insertions(+), 0 deletions(-)
 
 diff --git a/hw/pci_host.c b/hw/pci_host.c
 index 44c6c20..8041778 100644
 --- a/hw/pci_host.c
 +++ b/hw/pci_host.c
 @@ -101,6 +101,9 @@ static void pci_host_config_write(void *opaque, 
 target_phys_addr_t addr,
  
  PCI_DPRINTF(%s addr  TARGET_FMT_plx  len %d val %PRIx64\n,
  __func__, addr, len, val);
 +if (addr != 0 || len != 4) {
 +return;
 +}
  s-config_reg = val;
  }

Re: [Qemu-devel] [PATCH master/stable-1.0] pci: fix corrupted pci conf index register by unaligned write

2012-01-08 Thread Michael S. Tsirkin 
On Thu, Jan 05, 2012 at 04:14:29PM +0100, Stefan Weil wrote:
 Am 04.01.2012 15:47, schrieb Michael S. Tsirkin:
 On Wed, Jan 04, 2012 at 04:28:42PM +0200, Avi Kivity wrote:
 Commit d0ed8076cbdc261 converted the PCI config access to the memory
 API, but also inadvertantly changed it to accept unaligned writes,
 and corrupt the index register in the process. This causes a regression
 booting NetBSD.
 
 Fix by ignoring unaligned or non-dword writes.
 
 https://bugs.launchpad.net/qemu/+bug/897771
 
 Reported-by: Andreas Gustafsson g...@gson.org
 Signed-off-by: Avi Kivity a...@redhat.com
 
 Acked-by: Michael S. Tsirkin m...@redhat.com
 
 ---
 
 hw/pci_host.c | 3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
 
 diff --git a/hw/pci_host.c b/hw/pci_host.c
 index 44c6c20..8041778 100644
 --- a/hw/pci_host.c
 +++ b/hw/pci_host.c
 @@ -101,6 +101,9 @@ static void pci_host_config_write(void
 *opaque, target_phys_addr_t addr,
 
 PCI_DPRINTF(%s addr  TARGET_FMT_plx  len %d val %PRIx64\n,
 __func__, addr, len, val);
 + if (addr != 0 || len != 4) {
 + return;
 + }
 s-config_reg = val;
 }
 
 -- 
 1.7.7.1
 
 Non dword writes are quite common. I get them with Linux kernels, too.
 Do you really want to ignore them?

Are you sure?
Note this is an io write at cf8. Not an unaligned config write.

 And the check for unaligned writes is, well, unusual :-)

This seems to be how memory API behaves ... right, Avi?
Maybe this should be documented somewhere.

 Regards,
 Stefan

Re: [Qemu-devel] [PATCH master/stable-1.0] pci: fix corrupted pci conf index register by unaligned write

2012-01-08 Thread Avi Kivity 
On 01/08/2012 11:17 AM, Michael S. Tsirkin wrote:
 On Thu, Jan 05, 2012 at 04:14:29PM +0100, Stefan Weil wrote:
  Am 04.01.2012 15:47, schrieb Michael S. Tsirkin:
  On Wed, Jan 04, 2012 at 04:28:42PM +0200, Avi Kivity wrote:
  Commit d0ed8076cbdc261 converted the PCI config access to the memory
  API, but also inadvertantly changed it to accept unaligned writes,
  and corrupt the index register in the process. This causes a regression
  booting NetBSD.
  
  Fix by ignoring unaligned or non-dword writes.
  
  https://bugs.launchpad.net/qemu/+bug/897771
  
  Reported-by: Andreas Gustafsson g...@gson.org
  Signed-off-by: Avi Kivity a...@redhat.com
  
  Acked-by: Michael S. Tsirkin m...@redhat.com
  
  ---
  
  hw/pci_host.c | 3 +++
  1 files changed, 3 insertions(+), 0 deletions(-)
  
  diff --git a/hw/pci_host.c b/hw/pci_host.c
  index 44c6c20..8041778 100644
  --- a/hw/pci_host.c
  +++ b/hw/pci_host.c
  @@ -101,6 +101,9 @@ static void pci_host_config_write(void
  *opaque, target_phys_addr_t addr,
  
  PCI_DPRINTF(%s addr  TARGET_FMT_plx  len %d val %PRIx64\n,
  __func__, addr, len, val);
  + if (addr != 0 || len != 4) {
  + return;
  + }
  s-config_reg = val;
  }
  
  -- 
  1.7.7.1
  
  Non dword writes are quite common. I get them with Linux kernels, too.
  Do you really want to ignore them?

 Are you sure?
 Note this is an io write at cf8. Not an unaligned config write.

  And the check for unaligned writes is, well, unusual :-)

What's unusual?


 This seems to be how memory API behaves ... right, Avi?
 Maybe this should be documented somewhere.

Document what?

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH master/stable-1.0] pci: fix corrupted pci conf index register by unaligned write

2012-01-08 Thread Michael S. Tsirkin 
On Sun, Jan 08, 2012 at 12:02:35PM +0200, Avi Kivity wrote:
 On 01/08/2012 11:17 AM, Michael S. Tsirkin wrote:
  On Thu, Jan 05, 2012 at 04:14:29PM +0100, Stefan Weil wrote:
   Am 04.01.2012 15:47, schrieb Michael S. Tsirkin:
   On Wed, Jan 04, 2012 at 04:28:42PM +0200, Avi Kivity wrote:
   Commit d0ed8076cbdc261 converted the PCI config access to the memory
   API, but also inadvertantly changed it to accept unaligned writes,
   and corrupt the index register in the process. This causes a regression
   booting NetBSD.
   
   Fix by ignoring unaligned or non-dword writes.
   
   https://bugs.launchpad.net/qemu/+bug/897771
   
   Reported-by: Andreas Gustafsson g...@gson.org
   Signed-off-by: Avi Kivity a...@redhat.com
   
   Acked-by: Michael S. Tsirkin m...@redhat.com
   
   ---
   
   hw/pci_host.c | 3 +++
   1 files changed, 3 insertions(+), 0 deletions(-)
   
   diff --git a/hw/pci_host.c b/hw/pci_host.c
   index 44c6c20..8041778 100644
   --- a/hw/pci_host.c
   +++ b/hw/pci_host.c
   @@ -101,6 +101,9 @@ static void pci_host_config_write(void
   *opaque, target_phys_addr_t addr,
   
   PCI_DPRINTF(%s addr  TARGET_FMT_plx  len %d val %PRIx64\n,
   __func__, addr, len, val);
   + if (addr != 0 || len != 4) {
   + return;
   + }
   s-config_reg = val;
   }
   
   -- 
   1.7.7.1
   
   Non dword writes are quite common. I get them with Linux kernels, too.
   Do you really want to ignore them?
 
  Are you sure?
  Note this is an io write at cf8. Not an unaligned config write.
 
   And the check for unaligned writes is, well, unusual :-)
 
 What's unusual?
 
 
  This seems to be how memory API behaves ... right, Avi?
  Maybe this should be documented somewhere.
 
 Document what?

That address passed to callbacks is in fact an offset
from start of the region.

 -- 
 error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH master/stable-1.0] pci: fix corrupted pci conf index register by unaligned write

2012-01-08 Thread Avi Kivity 
On 01/08/2012 12:12 PM, Michael S. Tsirkin wrote:
  
And the check for unaligned writes is, well, unusual :-)
  
  What's unusual?
  
  
   This seems to be how memory API behaves ... right, Avi?
   Maybe this should be documented somewhere.
  
  Document what?

 That address passed to callbacks is in fact an offset
 from start of the region.


It's documented already (and it's not new - since 8da3ff18097, Dec 2008).

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH master/stable-1.0] pci: fix corrupted pci conf index register by unaligned write

2012-01-08 Thread Michael S. Tsirkin 
On Sun, Jan 08, 2012 at 12:48:05PM +0200, Avi Kivity wrote:
 On 01/08/2012 12:12 PM, Michael S. Tsirkin wrote:
   
 And the check for unaligned writes is, well, unusual :-)
   
   What's unusual?
   
   
This seems to be how memory API behaves ... right, Avi?
Maybe this should be documented somewhere.
   
   Document what?
 
  That address passed to callbacks is in fact an offset
  from start of the region.
 
 
 It's documented already (and it's not new - since 8da3ff18097, Dec 2008).

True, memory.h explicitly says:
@addr is relative to @mr

But now that's mentioned, I have a question:
we could also specify min access size and disable
unaligned access in ops.valid structure.
That would trigger machine checks on access errors
instead of ignoring them as we do now.
Anyone knows offhand what does real hardware do?

 -- 
 error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH master/stable-1.0] pci: fix corrupted pci conf index register by unaligned write

2012-01-08 Thread Avi Kivity 
On 01/08/2012 02:45 PM, Michael S. Tsirkin wrote:
 On Sun, Jan 08, 2012 at 12:48:05PM +0200, Avi Kivity wrote:
  On 01/08/2012 12:12 PM, Michael S. Tsirkin wrote:

  And the check for unaligned writes is, well, unusual :-)

What's unusual?


 This seems to be how memory API behaves ... right, Avi?
 Maybe this should be documented somewhere.

Document what?
  
   That address passed to callbacks is in fact an offset
   from start of the region.
  
  
  It's documented already (and it's not new - since 8da3ff18097, Dec 2008).

 True, memory.h explicitly says:
 @addr is relative to @mr

 But now that's mentioned, I have a question:
 we could also specify min access size and disable
 unaligned access in ops.valid structure.
 That would trigger machine checks on access errors
 instead of ignoring them as we do now.
 Anyone knows offhand what does real hardware do?

It depends on the actual hardware.  qemu has some #ifdefs deep down that
control this.

Ultimately this should be local to the bus rather than global; for
example a bus can intercept unaligned writes and invoke some bus
specific behaviour even though the cpu allows those accesses.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH master/stable-1.0] pci: fix corrupted pci conf index register by unaligned write

2012-01-07 Thread Stefan Weil 

Am 04.01.2012 15:47, schrieb Michael S. Tsirkin:

On Wed, Jan 04, 2012 at 04:28:42PM +0200, Avi Kivity wrote:

Commit d0ed8076cbdc261 converted the PCI config access to the memory
API, but also inadvertantly changed it to accept unaligned writes,
and corrupt the index register in the process. This causes a regression
booting NetBSD.

Fix by ignoring unaligned or non-dword writes.

https://bugs.launchpad.net/qemu/+bug/897771

Reported-by: Andreas Gustafsson g...@gson.org
Signed-off-by: Avi Kivity a...@redhat.com


Acked-by: Michael S. Tsirkin m...@redhat.com


---

hw/pci_host.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/pci_host.c b/hw/pci_host.c
index 44c6c20..8041778 100644
--- a/hw/pci_host.c
+++ b/hw/pci_host.c
@@ -101,6 +101,9 @@ static void pci_host_config_write(void *opaque, 
target_phys_addr_t addr,


PCI_DPRINTF(%s addr  TARGET_FMT_plx  len %d val %PRIx64\n,
__func__, addr, len, val);
+ if (addr != 0 || len != 4) {
+ return;
+ }
s-config_reg = val;
}

--
1.7.7.1


Non dword writes are quite common. I get them with Linux kernels, too.
Do you really want to ignore them?

And the check for unaligned writes is, well, unusual :-)

Regards,
Stefan

[Qemu-devel] [PATCH master/stable-1.0] pci: fix corrupted pci conf index register by unaligned write

2012-01-04 Thread Avi Kivity 
Commit d0ed8076cbdc261 converted the PCI config access to the memory
API, but also inadvertantly changed it to accept unaligned writes,
and corrupt the index register in the process.  This causes a regression
booting NetBSD.

Fix by ignoring unaligned or non-dword writes.

https://bugs.launchpad.net/qemu/+bug/897771

Reported-by: Andreas Gustafsson g...@gson.org
Signed-off-by: Avi Kivity a...@redhat.com
---

 hw/pci_host.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/pci_host.c b/hw/pci_host.c
index 44c6c20..8041778 100644
--- a/hw/pci_host.c
+++ b/hw/pci_host.c
@@ -101,6 +101,9 @@ static void pci_host_config_write(void *opaque, 
target_phys_addr_t addr,
 
 PCI_DPRINTF(%s addr  TARGET_FMT_plx  len %d val %PRIx64\n,
 __func__, addr, len, val);
+if (addr != 0 || len != 4) {
+return;
+}
 s-config_reg = val;
 }
 
-- 
1.7.7.1

Re: [Qemu-devel] [PATCH master/stable-1.0] pci: fix corrupted pci conf index register by unaligned write

2012-01-04 Thread Michael S. Tsirkin 
On Wed, Jan 04, 2012 at 04:28:42PM +0200, Avi Kivity wrote:
 Commit d0ed8076cbdc261 converted the PCI config access to the memory
 API, but also inadvertantly changed it to accept unaligned writes,
 and corrupt the index register in the process.  This causes a regression
 booting NetBSD.
 
 Fix by ignoring unaligned or non-dword writes.
 
 https://bugs.launchpad.net/qemu/+bug/897771
 
 Reported-by: Andreas Gustafsson g...@gson.org
 Signed-off-by: Avi Kivity a...@redhat.com

Acked-by: Michael S. Tsirkin m...@redhat.com

 ---
 
  hw/pci_host.c |3 +++
  1 files changed, 3 insertions(+), 0 deletions(-)
 
 diff --git a/hw/pci_host.c b/hw/pci_host.c
 index 44c6c20..8041778 100644
 --- a/hw/pci_host.c
 +++ b/hw/pci_host.c
 @@ -101,6 +101,9 @@ static void pci_host_config_write(void *opaque, 
 target_phys_addr_t addr,
  
  PCI_DPRINTF(%s addr  TARGET_FMT_plx  len %d val %PRIx64\n,
  __func__, addr, len, val);
 +if (addr != 0 || len != 4) {
 +return;
 +}
  s-config_reg = val;
  }
  
 -- 
 1.7.7.1

Re: [Qemu-devel] [PATCH master/stable-1.0] pci: fix corrupted pci conf index register by unaligned write

2012-01-04 Thread Alexander Graf 

On 04.01.2012, at 15:47, Michael S. Tsirkin wrote:

 On Wed, Jan 04, 2012 at 04:28:42PM +0200, Avi Kivity wrote:
 Commit d0ed8076cbdc261 converted the PCI config access to the memory
 API, but also inadvertantly changed it to accept unaligned writes,
 and corrupt the index register in the process.  This causes a regression
 booting NetBSD.
 
 Fix by ignoring unaligned or non-dword writes.
 
 https://bugs.launchpad.net/qemu/+bug/897771
 
 Reported-by: Andreas Gustafsson g...@gson.org
 Signed-off-by: Avi Kivity a...@redhat.com
 
 Acked-by: Michael S. Tsirkin m...@redhat.com

CC'ing qemu-stable.


Alex

 
 ---
 
 hw/pci_host.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
 
 diff --git a/hw/pci_host.c b/hw/pci_host.c
 index 44c6c20..8041778 100644
 --- a/hw/pci_host.c
 +++ b/hw/pci_host.c
 @@ -101,6 +101,9 @@ static void pci_host_config_write(void *opaque, 
 target_phys_addr_t addr,
 
 PCI_DPRINTF(%s addr  TARGET_FMT_plx  len %d val %PRIx64\n,
 __func__, addr, len, val);
 +if (addr != 0 || len != 4) {
 +return;
 +}
 s-config_reg = val;
 }
 
 -- 
 1.7.7.1