* [Qemu-devel] [PATCH] pci: w1cmask[PCI_BRIDGE_CONTROL] initialized incorrectly
@ 2011-01-26 9:45 Isaku Yamahata
2011-01-26 12:09 ` [Qemu-devel] " Michael S. Tsirkin
0 siblings, 1 reply; 7+ messages in thread
From: Isaku Yamahata @ 2011-01-26 9:45 UTC (permalink / raw)
To: qemu-devel; +Cc: yamahata, mst
pci_init_wmask_bridge() incorrectly set w1cmask[PCI_BRIDGE_CONTROL].
This patch removes the line otherwise the assert(!(wmask & w1cmask)) in
pci_default_write_config() is hit.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
hw/pci.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index b8f5385..79a46e7 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -643,10 +643,6 @@ static void pci_init_wmask_bridge(PCIDevice *d)
PCI_BRIDGE_CTL_SEC_DISCARD |
PCI_BRIDGE_CTL_DISCARD_STATUS |
PCI_BRIDGE_CTL_DISCARD_SERR);
- /* Below does not do anything as we never set this bit, put here for
- * completeness. */
- pci_set_word(d->w1cmask + PCI_BRIDGE_CONTROL,
- PCI_BRIDGE_CTL_DISCARD_STATUS);
}
static int pci_init_multifunction(PCIBus *bus, PCIDevice *dev)
--
1.7.1.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [Qemu-devel] Re: [PATCH] pci: w1cmask[PCI_BRIDGE_CONTROL] initialized incorrectly
2011-01-26 9:45 [Qemu-devel] [PATCH] pci: w1cmask[PCI_BRIDGE_CONTROL] initialized incorrectly Isaku Yamahata
@ 2011-01-26 12:09 ` Michael S. Tsirkin
2011-01-26 13:17 ` Isaku Yamahata
0 siblings, 1 reply; 7+ messages in thread
From: Michael S. Tsirkin @ 2011-01-26 12:09 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: qemu-devel
On Wed, Jan 26, 2011 at 06:45:27PM +0900, Isaku Yamahata wrote:
> pci_init_wmask_bridge() incorrectly set w1cmask[PCI_BRIDGE_CONTROL].
> This patch removes the line otherwise the assert(!(wmask & w1cmask)) in
> pci_default_write_config() is hit.
>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
Maybe clear in wmask? This bit really should be w1c, should it not?
> ---
> hw/pci.c | 4 ----
> 1 files changed, 0 insertions(+), 4 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index b8f5385..79a46e7 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -643,10 +643,6 @@ static void pci_init_wmask_bridge(PCIDevice *d)
> PCI_BRIDGE_CTL_SEC_DISCARD |
> PCI_BRIDGE_CTL_DISCARD_STATUS |
> PCI_BRIDGE_CTL_DISCARD_SERR);
> - /* Below does not do anything as we never set this bit, put here for
> - * completeness. */
> - pci_set_word(d->w1cmask + PCI_BRIDGE_CONTROL,
> - PCI_BRIDGE_CTL_DISCARD_STATUS);
> }
>
> static int pci_init_multifunction(PCIBus *bus, PCIDevice *dev)
> --
> 1.7.1.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] Re: [PATCH] pci: w1cmask[PCI_BRIDGE_CONTROL] initialized incorrectly
2011-01-26 12:09 ` [Qemu-devel] " Michael S. Tsirkin
@ 2011-01-26 13:17 ` Isaku Yamahata
2011-01-26 13:46 ` Michael S. Tsirkin
0 siblings, 1 reply; 7+ messages in thread
From: Isaku Yamahata @ 2011-01-26 13:17 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel
The bit should be writable, not w1c.
3.2.5.18 bridge control register
bit 11 Discard Timer SERR# Enable
When set to 1, this bit enables the bridge to assert SERR# on
the primary interface when either the Primary Discard Timer or
Secondary Discard Timer expires and a Delayed Transaction is
discarded from a queue in the bridge. The default state of this
bit must be 0 after reset.
0 - do not assert SERR# on the primary interface as
a result of the expiration of either the Primary
Discard Timer or Secondary Discard Timer
1 - assert SERR# on the primary interface if either
the Primary Discard Timer or Secondary Discard
Timer expires and a Delayed Transaction
On Wed, Jan 26, 2011 at 02:09:59PM +0200, Michael S. Tsirkin wrote:
> On Wed, Jan 26, 2011 at 06:45:27PM +0900, Isaku Yamahata wrote:
> > pci_init_wmask_bridge() incorrectly set w1cmask[PCI_BRIDGE_CONTROL].
> > This patch removes the line otherwise the assert(!(wmask & w1cmask)) in
> > pci_default_write_config() is hit.
> >
> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
>
> Maybe clear in wmask? This bit really should be w1c, should it not?
>
> > ---
> > hw/pci.c | 4 ----
> > 1 files changed, 0 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/pci.c b/hw/pci.c
> > index b8f5385..79a46e7 100644
> > --- a/hw/pci.c
> > +++ b/hw/pci.c
> > @@ -643,10 +643,6 @@ static void pci_init_wmask_bridge(PCIDevice *d)
> > PCI_BRIDGE_CTL_SEC_DISCARD |
> > PCI_BRIDGE_CTL_DISCARD_STATUS |
> > PCI_BRIDGE_CTL_DISCARD_SERR);
> > - /* Below does not do anything as we never set this bit, put here for
> > - * completeness. */
> > - pci_set_word(d->w1cmask + PCI_BRIDGE_CONTROL,
> > - PCI_BRIDGE_CTL_DISCARD_STATUS);
> > }
> >
> > static int pci_init_multifunction(PCIBus *bus, PCIDevice *dev)
> > --
> > 1.7.1.1
>
--
yamahata
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] Re: [PATCH] pci: w1cmask[PCI_BRIDGE_CONTROL] initialized incorrectly
2011-01-26 13:17 ` Isaku Yamahata
@ 2011-01-26 13:46 ` Michael S. Tsirkin
2011-01-26 13:53 ` Isaku Yamahata
0 siblings, 1 reply; 7+ messages in thread
From: Michael S. Tsirkin @ 2011-01-26 13:46 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: qemu-devel
On Wed, Jan 26, 2011 at 10:17:48PM +0900, Isaku Yamahata wrote:
> The bit should be writable, not w1c.
>
> 3.2.5.18 bridge control register
> bit 11 Discard Timer SERR# Enable
>
> When set to 1, this bit enables the bridge to assert SERR# on
> the primary interface when either the Primary Discard Timer or
> Secondary Discard Timer expires and a Delayed Transaction is
> discarded from a queue in the bridge. The default state of this
> bit must be 0 after reset.
> 0 - do not assert SERR# on the primary interface as
> a result of the expiration of either the Primary
> Discard Timer or Secondary Discard Timer
> 1 - assert SERR# on the primary interface if either
> the Primary Discard Timer or Secondary Discard
> Timer expires and a Delayed Transaction
Yes but
#define PCI_BRIDGE_CTL_DISCARD_STATUS 0x400 /* Discard timer status */
So this is bit 10?
BTW, how about moving these bits to linux and then to our pci_regs.h?
>S. Tsirkin wrote:
> > On Wed, Jan 26, 2011 at 06:45:27PM +0900, Isaku Yamahata wrote:
> > > pci_init_wmask_bridge() incorrectly set w1cmask[PCI_BRIDGE_CONTROL].
> > > This patch removes the line otherwise the assert(!(wmask & w1cmask)) in
> > > pci_default_write_config() is hit.
> > >
> > > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> >
> > Maybe clear in wmask? This bit really should be w1c, should it not?
> >
> > > ---
> > > hw/pci.c | 4 ----
> > > 1 files changed, 0 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/hw/pci.c b/hw/pci.c
> > > index b8f5385..79a46e7 100644
> > > --- a/hw/pci.c
> > > +++ b/hw/pci.c
> > > @@ -643,10 +643,6 @@ static void pci_init_wmask_bridge(PCIDevice *d)
> > > PCI_BRIDGE_CTL_SEC_DISCARD |
> > > PCI_BRIDGE_CTL_DISCARD_STATUS |
> > > PCI_BRIDGE_CTL_DISCARD_SERR);
> > > - /* Below does not do anything as we never set this bit, put here for
> > > - * completeness. */
> > > - pci_set_word(d->w1cmask + PCI_BRIDGE_CONTROL,
> > > - PCI_BRIDGE_CTL_DISCARD_STATUS);
> > > }
> > >
> > > static int pci_init_multifunction(PCIBus *bus, PCIDevice *dev)
> > > --
> > > 1.7.1.1
> >
>
> --
> yamahata
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] Re: [PATCH] pci: w1cmask[PCI_BRIDGE_CONTROL] initialized incorrectly
2011-01-26 13:46 ` Michael S. Tsirkin
@ 2011-01-26 13:53 ` Isaku Yamahata
2011-01-26 13:57 ` Michael S. Tsirkin
0 siblings, 1 reply; 7+ messages in thread
From: Isaku Yamahata @ 2011-01-26 13:53 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel
On Wed, Jan 26, 2011 at 03:46:01PM +0200, Michael S. Tsirkin wrote:
> On Wed, Jan 26, 2011 at 10:17:48PM +0900, Isaku Yamahata wrote:
> > The bit should be writable, not w1c.
> >
> > 3.2.5.18 bridge control register
> > bit 11 Discard Timer SERR# Enable
> >
> > When set to 1, this bit enables the bridge to assert SERR# on
> > the primary interface when either the Primary Discard Timer or
> > Secondary Discard Timer expires and a Delayed Transaction is
> > discarded from a queue in the bridge. The default state of this
> > bit must be 0 after reset.
> > 0 - do not assert SERR# on the primary interface as
> > a result of the expiration of either the Primary
> > Discard Timer or Secondary Discard Timer
> > 1 - assert SERR# on the primary interface if either
> > the Primary Discard Timer or Secondary Discard
> > Timer expires and a Delayed Transaction
>
> Yes but
> #define PCI_BRIDGE_CTL_DISCARD_STATUS 0x400 /* Discard timer status */
>
> So this is bit 10?
Oh, sorry. So wmask should be chenged.
> BTW, how about moving these bits to linux and then to our pci_regs.h?
>
> >S. Tsirkin wrote:
> > > On Wed, Jan 26, 2011 at 06:45:27PM +0900, Isaku Yamahata wrote:
> > > > pci_init_wmask_bridge() incorrectly set w1cmask[PCI_BRIDGE_CONTROL].
> > > > This patch removes the line otherwise the assert(!(wmask & w1cmask)) in
> > > > pci_default_write_config() is hit.
> > > >
> > > > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> > >
> > > Maybe clear in wmask? This bit really should be w1c, should it not?
> > >
> > > > ---
> > > > hw/pci.c | 4 ----
> > > > 1 files changed, 0 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/hw/pci.c b/hw/pci.c
> > > > index b8f5385..79a46e7 100644
> > > > --- a/hw/pci.c
> > > > +++ b/hw/pci.c
> > > > @@ -643,10 +643,6 @@ static void pci_init_wmask_bridge(PCIDevice *d)
> > > > PCI_BRIDGE_CTL_SEC_DISCARD |
> > > > PCI_BRIDGE_CTL_DISCARD_STATUS |
> > > > PCI_BRIDGE_CTL_DISCARD_SERR);
> > > > - /* Below does not do anything as we never set this bit, put here for
> > > > - * completeness. */
> > > > - pci_set_word(d->w1cmask + PCI_BRIDGE_CONTROL,
> > > > - PCI_BRIDGE_CTL_DISCARD_STATUS);
> > > > }
> > > >
> > > > static int pci_init_multifunction(PCIBus *bus, PCIDevice *dev)
> > > > --
> > > > 1.7.1.1
> > >
> >
> > --
> > yamahata
>
--
yamahata
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] Re: [PATCH] pci: w1cmask[PCI_BRIDGE_CONTROL] initialized incorrectly
2011-01-26 13:53 ` Isaku Yamahata
@ 2011-01-26 13:57 ` Michael S. Tsirkin
2011-01-27 3:54 ` Isaku Yamahata
0 siblings, 1 reply; 7+ messages in thread
From: Michael S. Tsirkin @ 2011-01-26 13:57 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: qemu-devel
On Wed, Jan 26, 2011 at 10:53:42PM +0900, Isaku Yamahata wrote:
> On Wed, Jan 26, 2011 at 03:46:01PM +0200, Michael S. Tsirkin wrote:
> > On Wed, Jan 26, 2011 at 10:17:48PM +0900, Isaku Yamahata wrote:
> > > The bit should be writable, not w1c.
> > >
> > > 3.2.5.18 bridge control register
> > > bit 11 Discard Timer SERR# Enable
> > >
> > > When set to 1, this bit enables the bridge to assert SERR# on
> > > the primary interface when either the Primary Discard Timer or
> > > Secondary Discard Timer expires and a Delayed Transaction is
> > > discarded from a queue in the bridge. The default state of this
> > > bit must be 0 after reset.
> > > 0 - do not assert SERR# on the primary interface as
> > > a result of the expiration of either the Primary
> > > Discard Timer or Secondary Discard Timer
> > > 1 - assert SERR# on the primary interface if either
> > > the Primary Discard Timer or Secondary Discard
> > > Timer expires and a Delayed Transaction
> >
> > Yes but
> > #define PCI_BRIDGE_CTL_DISCARD_STATUS 0x400 /* Discard timer status */
> >
> > So this is bit 10?
>
> Oh, sorry. So wmask should be chenged.
Yes. does the below work?
commit 24ebb78e6ec19b39cd138d493a2859122110f9cb
Author: Michael S. Tsirkin <mst@redhat.com>
Date: Wed Jan 26 15:55:07 2011 +0200
pci: bridge control fixup
PCI_BRIDGE_CTL_DISCARD_STATUS (bit 10 in bridge control register)
is W1C so we should not make it writeable, otherwise the assert(!(wmask
& w1cmask)) in pci_default_write_config() is hit
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reported-by: Isaku Yamahata <yamahata@valinux.co.jp>
diff --git a/hw/pci.c b/hw/pci.c
index 044c4bd..712280a 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -641,7 +641,6 @@ static void pci_init_wmask_bridge(PCIDevice *d)
PCI_BRIDGE_CTL_FAST_BACK |
PCI_BRIDGE_CTL_DISCARD |
PCI_BRIDGE_CTL_SEC_DISCARD |
- PCI_BRIDGE_CTL_DISCARD_STATUS |
PCI_BRIDGE_CTL_DISCARD_SERR);
/* Below does not do anything as we never set this bit, put here for
* completeness. */
^ permalink raw reply related [flat|nested] 7+ messages in thread* [Qemu-devel] Re: [PATCH] pci: w1cmask[PCI_BRIDGE_CONTROL] initialized incorrectly
2011-01-26 13:57 ` Michael S. Tsirkin
@ 2011-01-27 3:54 ` Isaku Yamahata
0 siblings, 0 replies; 7+ messages in thread
From: Isaku Yamahata @ 2011-01-27 3:54 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel
On Wed, Jan 26, 2011 at 03:57:15PM +0200, Michael S. Tsirkin wrote:
> On Wed, Jan 26, 2011 at 10:53:42PM +0900, Isaku Yamahata wrote:
> > On Wed, Jan 26, 2011 at 03:46:01PM +0200, Michael S. Tsirkin wrote:
> > > On Wed, Jan 26, 2011 at 10:17:48PM +0900, Isaku Yamahata wrote:
> > > > The bit should be writable, not w1c.
> > > >
> > > > 3.2.5.18 bridge control register
> > > > bit 11 Discard Timer SERR# Enable
> > > >
> > > > When set to 1, this bit enables the bridge to assert SERR# on
> > > > the primary interface when either the Primary Discard Timer or
> > > > Secondary Discard Timer expires and a Delayed Transaction is
> > > > discarded from a queue in the bridge. The default state of this
> > > > bit must be 0 after reset.
> > > > 0 - do not assert SERR# on the primary interface as
> > > > a result of the expiration of either the Primary
> > > > Discard Timer or Secondary Discard Timer
> > > > 1 - assert SERR# on the primary interface if either
> > > > the Primary Discard Timer or Secondary Discard
> > > > Timer expires and a Delayed Transaction
> > >
> > > Yes but
> > > #define PCI_BRIDGE_CTL_DISCARD_STATUS 0x400 /* Discard timer status */
> > >
> > > So this is bit 10?
> >
> > Oh, sorry. So wmask should be chenged.
>
> Yes. does the below work?
Yep. Thank you.
Tested-by: Isaku Yamahata <yamahata@valinux.co.jp>
>
> commit 24ebb78e6ec19b39cd138d493a2859122110f9cb
> Author: Michael S. Tsirkin <mst@redhat.com>
> Date: Wed Jan 26 15:55:07 2011 +0200
>
> pci: bridge control fixup
>
> PCI_BRIDGE_CTL_DISCARD_STATUS (bit 10 in bridge control register)
> is W1C so we should not make it writeable, otherwise the assert(!(wmask
> & w1cmask)) in pci_default_write_config() is hit
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Reported-by: Isaku Yamahata <yamahata@valinux.co.jp>
>
> diff --git a/hw/pci.c b/hw/pci.c
> index 044c4bd..712280a 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -641,7 +641,6 @@ static void pci_init_wmask_bridge(PCIDevice *d)
> PCI_BRIDGE_CTL_FAST_BACK |
> PCI_BRIDGE_CTL_DISCARD |
> PCI_BRIDGE_CTL_SEC_DISCARD |
> - PCI_BRIDGE_CTL_DISCARD_STATUS |
> PCI_BRIDGE_CTL_DISCARD_SERR);
> /* Below does not do anything as we never set this bit, put here for
> * completeness. */
>
--
yamahata
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-01-27 3:54 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-01-26 9:45 [Qemu-devel] [PATCH] pci: w1cmask[PCI_BRIDGE_CONTROL] initialized incorrectly Isaku Yamahata
2011-01-26 12:09 ` [Qemu-devel] " Michael S. Tsirkin
2011-01-26 13:17 ` Isaku Yamahata
2011-01-26 13:46 ` Michael S. Tsirkin
2011-01-26 13:53 ` Isaku Yamahata
2011-01-26 13:57 ` Michael S. Tsirkin
2011-01-27 3:54 ` Isaku Yamahata
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).