qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] bonito: PCI_STATUS questions
@ 2010-10-27 16:12 Michael S. Tsirkin
  2010-10-28  0:57 ` [Qemu-devel] " chen huacai
  0 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2010-10-27 16:12 UTC (permalink / raw)
  To: qemu-devel, zltjiangshi, Aurelien Jarno

I see code in bonito.c that clears bits:
PCI_STATUS_REC_MASTER_ABORT | PCI_STATUS_REC_TARGET_ABORT
on each read and write.

However
1. I don't see anything in code that would set these bits
2. The PCI spec says this about the status register:

	Reads to this register behave normally. Writes are slightly different in
	that bits can be reset, but not set. A one bit is reset (if it is not
	read-only) whenever the register is written, and the write data in the
	corresponding bit location is a 1. For instance, to clear bit 14 and not
	affect any other bits, write the value 0100_0000_0000_0000b to the
	register.

while the code in bonito.c resets the bits to 0 on each write.

Comments?

-- 
MST

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Qemu-devel] Re: bonito: PCI_STATUS questions
  2010-10-27 16:12 [Qemu-devel] bonito: PCI_STATUS questions Michael S. Tsirkin
@ 2010-10-28  0:57 ` chen huacai
  2010-10-28  4:54   ` Michael S. Tsirkin
  0 siblings, 1 reply; 6+ messages in thread
From: chen huacai @ 2010-10-28  0:57 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel, Aurelien Jarno

Because the code in PMON and Linux kernel use these bits to verify r/w
operations. If one of them is 1 after r/w, PMON and Linux will
consider r/w has failed.
I guess that software will not set them to 1, because it is set by
hardware when operation fails.

On Thu, Oct 28, 2010 at 12:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> I see code in bonito.c that clears bits:
> PCI_STATUS_REC_MASTER_ABORT | PCI_STATUS_REC_TARGET_ABORT
> on each read and write.
>
> However
> 1. I don't see anything in code that would set these bits
> 2. The PCI spec says this about the status register:
>
>        Reads to this register behave normally. Writes are slightly different in
>        that bits can be reset, but not set. A one bit is reset (if it is not
>        read-only) whenever the register is written, and the write data in the
>        corresponding bit location is a 1. For instance, to clear bit 14 and not
>        affect any other bits, write the value 0100_0000_0000_0000b to the
>        register.
>
> while the code in bonito.c resets the bits to 0 on each write.
>
> Comments?
>
> --
> MST
>



-- 
Huacai Chen

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Qemu-devel] Re: bonito: PCI_STATUS questions
  2010-10-28  0:57 ` [Qemu-devel] " chen huacai
@ 2010-10-28  4:54   ` Michael S. Tsirkin
  2010-10-28  8:31     ` chen huacai
  0 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2010-10-28  4:54 UTC (permalink / raw)
  To: chen huacai; +Cc: qemu-devel, Aurelien Jarno

On Thu, Oct 28, 2010 at 08:57:01AM +0800, chen huacai wrote:
> Because the code in PMON and Linux kernel use these bits to verify r/w
> operations. If one of them is 1 after r/w, PMON and Linux will
> consider r/w has failed.

Where's that code in Linux?

> I guess that software will not set them to 1, because it is set by
> hardware when operation fails.

So I guess just making these write 1 to clear according to spec will work?

> On Thu, Oct 28, 2010 at 12:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > I see code in bonito.c that clears bits:
> > PCI_STATUS_REC_MASTER_ABORT | PCI_STATUS_REC_TARGET_ABORT
> > on each read and write.
> >
> > However
> > 1. I don't see anything in code that would set these bits
> > 2. The PCI spec says this about the status register:
> >
> >        Reads to this register behave normally. Writes are slightly different in
> >        that bits can be reset, but not set. A one bit is reset (if it is not
> >        read-only) whenever the register is written, and the write data in the
> >        corresponding bit location is a 1. For instance, to clear bit 14 and not
> >        affect any other bits, write the value 0100_0000_0000_0000b to the
> >        register.
> >
> > while the code in bonito.c resets the bits to 0 on each write.
> >
> > Comments?
> >
> > --
> > MST
> >
> 
> 
> 
> -- 
> Huacai Chen

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Qemu-devel] Re: bonito: PCI_STATUS questions
  2010-10-28  4:54   ` Michael S. Tsirkin
@ 2010-10-28  8:31     ` chen huacai
  2010-10-29  8:16       ` Michael S. Tsirkin
  0 siblings, 1 reply; 6+ messages in thread
From: chen huacai @ 2010-10-28  8:31 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel, Aurelien Jarno

Please see bonito64_pcibios_config_access() in
arch/mips/pci/ops-bonito64.c of Linux kernel code.
You will find something useful.

On Thu, Oct 28, 2010 at 12:54 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Thu, Oct 28, 2010 at 08:57:01AM +0800, chen huacai wrote:
>> Because the code in PMON and Linux kernel use these bits to verify r/w
>> operations. If one of them is 1 after r/w, PMON and Linux will
>> consider r/w has failed.
>
> Where's that code in Linux?
>
>> I guess that software will not set them to 1, because it is set by
>> hardware when operation fails.
>
> So I guess just making these write 1 to clear according to spec will work?
>
>> On Thu, Oct 28, 2010 at 12:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > I see code in bonito.c that clears bits:
>> > PCI_STATUS_REC_MASTER_ABORT | PCI_STATUS_REC_TARGET_ABORT
>> > on each read and write.
>> >
>> > However
>> > 1. I don't see anything in code that would set these bits
>> > 2. The PCI spec says this about the status register:
>> >
>> >        Reads to this register behave normally. Writes are slightly different in
>> >        that bits can be reset, but not set. A one bit is reset (if it is not
>> >        read-only) whenever the register is written, and the write data in the
>> >        corresponding bit location is a 1. For instance, to clear bit 14 and not
>> >        affect any other bits, write the value 0100_0000_0000_0000b to the
>> >        register.
>> >
>> > while the code in bonito.c resets the bits to 0 on each write.
>> >
>> > Comments?
>> >
>> > --
>> > MST
>> >
>>
>>
>>
>> --
>> Huacai Chen
>



-- 
Huacai Chen

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Qemu-devel] Re: bonito: PCI_STATUS questions
  2010-10-28  8:31     ` chen huacai
@ 2010-10-29  8:16       ` Michael S. Tsirkin
  2010-10-29 13:34         ` chen huacai
  0 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2010-10-29  8:16 UTC (permalink / raw)
  To: chen huacai; +Cc: qemu-devel, Aurelien Jarno

On Thu, Oct 28, 2010 at 04:31:33PM +0800, chen huacai wrote:
> Please see bonito64_pcibios_config_access() in
> arch/mips/pci/ops-bonito64.c of Linux kernel code.
> You will find something useful.


Thanks!
I found this:

        /* Clear aborts */
        BONITO_PCICMD |= BONITO_PCICMD_MABORT_CLR | BONITO_PCICMD_MTABORT_CLR;

        BONITO_PCIMAP_CFG = (addr >> 16) | type;

        /* Flush Bonito register block */
        dummy = BONITO_PCIMAP_CFG;
        mmiowb();

        addrp = CFG_SPACE_REG(addr & 0xffff);
        if (access_type == PCI_ACCESS_WRITE) {
                writel(cpu_to_le32(*data), addrp);
                /* Wait till done */
                while (BONITO_PCIMSTAT & 0xF);
        } else {
                *data = le32_to_cpu(readl(addrp));
        }

        /* Detect Master/Target abort */
        if (BONITO_PCICMD & (BONITO_PCICMD_MABORT_CLR |
                             BONITO_PCICMD_MTABORT_CLR)) {
                /* Error occurred */

                /* Clear bits */
                BONITO_PCICMD |= (BONITO_PCICMD_MABORT_CLR |
                                  BONITO_PCICMD_MTABORT_CLR);

                return -1;
        }

So it looks like guest will clear bits, perform a transaction
then check error bits. In other words, it looks like it
assumes that writing 1 to these bits will clear them.
Correct?

> On Thu, Oct 28, 2010 at 12:54 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Thu, Oct 28, 2010 at 08:57:01AM +0800, chen huacai wrote:
> >> Because the code in PMON and Linux kernel use these bits to verify r/w
> >> operations. If one of them is 1 after r/w, PMON and Linux will
> >> consider r/w has failed.
> >
> > Where's that code in Linux?
> >
> >> I guess that software will not set them to 1, because it is set by
> >> hardware when operation fails.
> >
> > So I guess just making these write 1 to clear according to spec will work?
> >
> >> On Thu, Oct 28, 2010 at 12:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > I see code in bonito.c that clears bits:
> >> > PCI_STATUS_REC_MASTER_ABORT | PCI_STATUS_REC_TARGET_ABORT
> >> > on each read and write.
> >> >
> >> > However
> >> > 1. I don't see anything in code that would set these bits
> >> > 2. The PCI spec says this about the status register:
> >> >
> >> >        Reads to this register behave normally. Writes are slightly different in
> >> >        that bits can be reset, but not set. A one bit is reset (if it is not
> >> >        read-only) whenever the register is written, and the write data in the
> >> >        corresponding bit location is a 1. For instance, to clear bit 14 and not
> >> >        affect any other bits, write the value 0100_0000_0000_0000b to the
> >> >        register.
> >> >
> >> > while the code in bonito.c resets the bits to 0 on each write.
> >> >
> >> > Comments?
> >> >
> >> > --
> >> > MST
> >> >
> >>
> >>
> >>
> >> --
> >> Huacai Chen
> >
> 
> 
> 
> -- 
> Huacai Chen

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Qemu-devel] Re: bonito: PCI_STATUS questions
  2010-10-29  8:16       ` Michael S. Tsirkin
@ 2010-10-29 13:34         ` chen huacai
  0 siblings, 0 replies; 6+ messages in thread
From: chen huacai @ 2010-10-29 13:34 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel, Aurelien Jarno

Yes, I think so.

On Fri, Oct 29, 2010 at 4:16 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Thu, Oct 28, 2010 at 04:31:33PM +0800, chen huacai wrote:
>> Please see bonito64_pcibios_config_access() in
>> arch/mips/pci/ops-bonito64.c of Linux kernel code.
>> You will find something useful.
>
>
> Thanks!
> I found this:
>
>        /* Clear aborts */
>        BONITO_PCICMD |= BONITO_PCICMD_MABORT_CLR | BONITO_PCICMD_MTABORT_CLR;
>
>        BONITO_PCIMAP_CFG = (addr >> 16) | type;
>
>        /* Flush Bonito register block */
>        dummy = BONITO_PCIMAP_CFG;
>        mmiowb();
>
>        addrp = CFG_SPACE_REG(addr & 0xffff);
>        if (access_type == PCI_ACCESS_WRITE) {
>                writel(cpu_to_le32(*data), addrp);
>                /* Wait till done */
>                while (BONITO_PCIMSTAT & 0xF);
>        } else {
>                *data = le32_to_cpu(readl(addrp));
>        }
>
>        /* Detect Master/Target abort */
>        if (BONITO_PCICMD & (BONITO_PCICMD_MABORT_CLR |
>                             BONITO_PCICMD_MTABORT_CLR)) {
>                /* Error occurred */
>
>                /* Clear bits */
>                BONITO_PCICMD |= (BONITO_PCICMD_MABORT_CLR |
>                                  BONITO_PCICMD_MTABORT_CLR);
>
>                return -1;
>        }
>
> So it looks like guest will clear bits, perform a transaction
> then check error bits. In other words, it looks like it
> assumes that writing 1 to these bits will clear them.
> Correct?
>
>> On Thu, Oct 28, 2010 at 12:54 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > On Thu, Oct 28, 2010 at 08:57:01AM +0800, chen huacai wrote:
>> >> Because the code in PMON and Linux kernel use these bits to verify r/w
>> >> operations. If one of them is 1 after r/w, PMON and Linux will
>> >> consider r/w has failed.
>> >
>> > Where's that code in Linux?
>> >
>> >> I guess that software will not set them to 1, because it is set by
>> >> hardware when operation fails.
>> >
>> > So I guess just making these write 1 to clear according to spec will work?
>> >
>> >> On Thu, Oct 28, 2010 at 12:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> >> > I see code in bonito.c that clears bits:
>> >> > PCI_STATUS_REC_MASTER_ABORT | PCI_STATUS_REC_TARGET_ABORT
>> >> > on each read and write.
>> >> >
>> >> > However
>> >> > 1. I don't see anything in code that would set these bits
>> >> > 2. The PCI spec says this about the status register:
>> >> >
>> >> >        Reads to this register behave normally. Writes are slightly different in
>> >> >        that bits can be reset, but not set. A one bit is reset (if it is not
>> >> >        read-only) whenever the register is written, and the write data in the
>> >> >        corresponding bit location is a 1. For instance, to clear bit 14 and not
>> >> >        affect any other bits, write the value 0100_0000_0000_0000b to the
>> >> >        register.
>> >> >
>> >> > while the code in bonito.c resets the bits to 0 on each write.
>> >> >
>> >> > Comments?
>> >> >
>> >> > --
>> >> > MST
>> >> >
>> >>
>> >>
>> >>
>> >> --
>> >> Huacai Chen
>> >
>>
>>
>>
>> --
>> Huacai Chen
>



-- 
Huacai Chen

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-10-29 13:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-27 16:12 [Qemu-devel] bonito: PCI_STATUS questions Michael S. Tsirkin
2010-10-28  0:57 ` [Qemu-devel] " chen huacai
2010-10-28  4:54   ` Michael S. Tsirkin
2010-10-28  8:31     ` chen huacai
2010-10-29  8:16       ` Michael S. Tsirkin
2010-10-29 13:34         ` chen huacai

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).