[Qemu-devel] questions on default_config_write in hw/pci.c

[Qemu-devel] questions on default_config_write in hw/pci.c

[Michael S. Tsirkin]

Hi,
I've been looking at hw/pci.c, specifically at how config
read/write are implemented, and have a couple of questions
about default_config_write:

1. The code at the beginning (if len == 4 ...)
   seems to only update pci base registers if a dword write 
   is performed. I think it's legal for the guest to perform 4
   single-byte writes. Should this be supported?

2. The large switch statement at the end of this function
   uses hard-coded register offsets. Would it make sense
   to change it to use macros from hw/pci.h?

3. Still there I see:
        switch(d->config[0x0e]) {
        case 0x00:
        case 0x80:
   register 0x0e is header type, which has defined values
   of 0x00 (device or host bridge), 0x01 (pci to pci bridge) and
   0x02 (cardbus bridge). What is 0x80 and when is it used?
   Would it make sense to remove this?

4. Still there, there's some handling done for type 1 devices.
   This support seems imcomplete.
   Are there any PCI-to-PCI bridges emulated by qemu?
   Would it make sense to remove this code?

Thanks,
MST

-- 
MST

[Qemu-devel] Re: questions on default_config_write in hw/pci.c

[Anthony Liguori]

I can only answer some of this..

Michael S. Tsirkin wrote:
> Hi,
> I've been looking at hw/pci.c, specifically at how config
> read/write are implemented, and have a couple of questions
> about default_config_write:
>
> 1. The code at the beginning (if len == 4 ...)
>    seems to only update pci base registers if a dword write 
>    is performed. I think it's legal for the guest to perform 4
>    single-byte writes. Should this be supported?
>
> 2. The large switch statement at the end of this function
>    uses hard-coded register offsets. Would it make sense
>    to change it to use macros from hw/pci.h?
>   

Yes.

> 3. Still there I see:
>         switch(d->config[0x0e]) {
>         case 0x00:
>         case 0x80:
>    register 0x0e is header type, which has defined values
>    of 0x00 (device or host bridge), 0x01 (pci to pci bridge) and
>    0x02 (cardbus bridge). What is 0x80 and when is it used?
>    Would it make sense to remove this?
>
> 4. Still there, there's some handling done for type 1 devices.
>    This support seems imcomplete.
>    Are there any PCI-to-PCI bridges emulated by qemu?
>    Would it make sense to remove this code?
>   

PCI-to-PCI bridges are used on non-x86 machine types by default.  
Presumably, they work enough for those boards.  I've tried to use a PCI 
bridge in an x86 machine type but it's not very functional (IIRC, it 
didn't work for devices in any slot other than 0).

-- 
Regards,

Anthony Liguori

[Qemu-devel] Re: questions on default_config_write in hw/pci.c

[Marcelo Tosatti]

On Thu, Apr 30, 2009 at 07:15:08PM +0300, Michael S. Tsirkin wrote:
> Hi,
> I've been looking at hw/pci.c, specifically at how config
> read/write are implemented, and have a couple of questions
> about default_config_write:
> 
> 1. The code at the beginning (if len == 4 ...)
>    seems to only update pci base registers if a dword write 
>    is performed. I think it's legal for the guest to perform 4
>    single-byte writes. Should this be supported?
> 
> 2. The large switch statement at the end of this function
>    uses hard-coded register offsets. Would it make sense
>    to change it to use macros from hw/pci.h?
> 
> 3. Still there I see:
>         switch(d->config[0x0e]) {
>         case 0x00:
>         case 0x80:
>    register 0x0e is header type, which has defined values
>    of 0x00 (device or host bridge), 0x01 (pci to pci bridge) and
>    0x02 (cardbus bridge). What is 0x80 and when is it used?
>    Would it make sense to remove this?

Don't know. Check the PCI spec?

> 4. Still there, there's some handling done for type 1 devices.
>    This support seems imcomplete.
>    Are there any PCI-to-PCI bridges emulated by qemu?
>    Would it make sense to remove this code?


It did work at one point:

http://www.mail-archive.com/kvm-devel@lists.sourceforge.net/msg16647.html

Re: [Qemu-devel] Re: questions on default_config_write in hw/pci.c

[M. Warner Losh]

In message: <20090430171526.GA10638@amt.cnet>
            Marcelo Tosatti <mtosatti@redhat.com> writes:
: On Thu, Apr 30, 2009 at 07:15:08PM +0300, Michael S. Tsirkin wrote:
: > Hi,
: > I've been looking at hw/pci.c, specifically at how config
: > read/write are implemented, and have a couple of questions
: > about default_config_write:
: > 
: > 1. The code at the beginning (if len == 4 ...)
: >    seems to only update pci base registers if a dword write 
: >    is performed. I think it's legal for the guest to perform 4
: >    single-byte writes. Should this be supported?
: > 
: > 2. The large switch statement at the end of this function
: >    uses hard-coded register offsets. Would it make sense
: >    to change it to use macros from hw/pci.h?
: > 
: > 3. Still there I see:
: >         switch(d->config[0x0e]) {
: >         case 0x00:
: >         case 0x80:
: >    register 0x0e is header type, which has defined values
: >    of 0x00 (device or host bridge), 0x01 (pci to pci bridge) and
: >    0x02 (cardbus bridge). What is 0x80 and when is it used?
: >    Would it make sense to remove this?
: 
: Don't know. Check the PCI spec?

The above is a bug.  0x80 is a bit that says "I have multiple
functions" and is orthogonal to the type of device.  

: >         switch(d->config[0x0e]) {

should be

	switch(d->config[0x0e] & 0x7f) {

: > 4. Still there, there's some handling done for type 1 devices.
: >    This support seems imcomplete.
: >    Are there any PCI-to-PCI bridges emulated by qemu?
: >    Would it make sense to remove this code?
: 
: 
: It did work at one point:
: 
: http://www.mail-archive.com/kvm-devel@lists.sourceforge.net/msg16647.html

I have vague plans to do Cardbus-to-PCI bridge at some point...

Warner

[Qemu-devel] Re: questions on default_config_write in hw/pci.c

[Michael S. Tsirkin]

On Thu, Apr 30, 2009 at 02:15:26PM -0300, Marcelo Tosatti wrote:
> On Thu, Apr 30, 2009 at 07:15:08PM +0300, Michael S. Tsirkin wrote:
> > Hi,
> > I've been looking at hw/pci.c, specifically at how config
> > read/write are implemented, and have a couple of questions
> > about default_config_write:
> > 
> > 1. The code at the beginning (if len == 4 ...)
> >    seems to only update pci base registers if a dword write 
> >    is performed. I think it's legal for the guest to perform 4
> >    single-byte writes. Should this be supported?
> > 
> > 2. The large switch statement at the end of this function
> >    uses hard-coded register offsets. Would it make sense
> >    to change it to use macros from hw/pci.h?
> > 
> > 3. Still there I see:
> >         switch(d->config[0x0e]) {
> >         case 0x00:
> >         case 0x80:
> >    register 0x0e is header type, which has defined values
> >    of 0x00 (device or host bridge), 0x01 (pci to pci bridge) and
> >    0x02 (cardbus bridge). What is 0x80 and when is it used?
> >    Would it make sense to remove this?
> 
> Don't know. Check the PCI spec?

That's the first thing I did :) What I site above is from there:
"The Header Type field (located at offset 0Eh) defines what layout is
provided. Currently three Header Types are defined, 00h which has the
layout shown in Figure 6-1, 01h which is defined for PCI-to-PCI bridges
and is documented in the PCI to PCI Bridge Architecture Specification,
and 02h which is defined for CardBus bridges and is documented in the PC
Card Standard."

> > 4. Still there, there's some handling done for type 1 devices.
> >    This support seems imcomplete.
> >    Are there any PCI-to-PCI bridges emulated by qemu?
> >    Would it make sense to remove this code?
> 
> 
> It did work at one point:
> 
> http://www.mail-archive.com/kvm-devel@lists.sourceforge.net/msg16647.html

Sigh. I guess I'll have to fix it rather than just rip it out then ...
What I refer to first of all is that writes to BAR registers go through
logic for regular PCI devices, which have a different layout.
Does this make sense for some reason?

-- 
MST

Re: [Qemu-devel] Re: questions on default_config_write in hw/pci.c

[Michael S. Tsirkin]

On Thu, Apr 30, 2009 at 11:28:20AM -0600, M. Warner Losh wrote:
> The above is a bug.  0x80 is a bit that says "I have multiple
> functions" and is orthogonal to the type of device.  

Ah. Thanks.

-- 
MST

[Qemu-devel] Re: questions on default_config_write in hw/pci.c

[Anthony Liguori]

Michael S. Tsirkin wrote:
>>> 4. Still there, there's some handling done for type 1 devices.
>>>    This support seems imcomplete.
>>>    Are there any PCI-to-PCI bridges emulated by qemu?
>>>    Would it make sense to remove this code?
>>>       
>> It did work at one point:
>>
>> http://www.mail-archive.com/kvm-devel@lists.sourceforge.net/msg16647.html
>>     
>
> Sigh. I guess I'll have to fix it rather than just rip it out then ...
> What I refer to first of all is that writes to BAR registers go through
> logic for regular PCI devices, which have a different layout.
> Does this make sense for some reason?
>
>   

IIRC, it didn't actually work as expected.  Marcelo?


-- 
Regards,

Anthony Liguori

[Qemu-devel] Re: questions on default_config_write in hw/pci.c

[Marcelo Tosatti]

On Thu, Apr 30, 2009 at 08:29:55PM +0300, Michael S. Tsirkin wrote:
> On Thu, Apr 30, 2009 at 02:15:26PM -0300, Marcelo Tosatti wrote:
> > On Thu, Apr 30, 2009 at 07:15:08PM +0300, Michael S. Tsirkin wrote:
> > > Hi,
> > > I've been looking at hw/pci.c, specifically at how config
> > > read/write are implemented, and have a couple of questions
> > > about default_config_write:
> > > 
> > > 1. The code at the beginning (if len == 4 ...)
> > >    seems to only update pci base registers if a dword write 
> > >    is performed. I think it's legal for the guest to perform 4
> > >    single-byte writes. Should this be supported?
> > > 
> > > 2. The large switch statement at the end of this function
> > >    uses hard-coded register offsets. Would it make sense
> > >    to change it to use macros from hw/pci.h?
> > > 
> > > 3. Still there I see:
> > >         switch(d->config[0x0e]) {
> > >         case 0x00:
> > >         case 0x80:
> > >    register 0x0e is header type, which has defined values
> > >    of 0x00 (device or host bridge), 0x01 (pci to pci bridge) and
> > >    0x02 (cardbus bridge). What is 0x80 and when is it used?
> > >    Would it make sense to remove this?
> > 
> > Don't know. Check the PCI spec?
> 
> That's the first thing I did :) What I site above is from there:
> "The Header Type field (located at offset 0Eh) defines what layout is
> provided. Currently three Header Types are defined, 00h which has the
> layout shown in Figure 6-1, 01h which is defined for PCI-to-PCI bridges
> and is documented in the PCI to PCI Bridge Architecture Specification,
> and 02h which is defined for CardBus bridges and is documented in the PC
> Card Standard."
> 
> > > 4. Still there, there's some handling done for type 1 devices.
> > >    This support seems imcomplete.
> > >    Are there any PCI-to-PCI bridges emulated by qemu?
> > >    Would it make sense to remove this code?
> > 
> > 
> > It did work at one point:
> > 
> > http://www.mail-archive.com/kvm-devel@lists.sourceforge.net/msg16647.html
> 
> Sigh. I guess I'll have to fix it rather than just rip it out then ...
> What I refer to first of all is that writes to BAR registers go through
> logic for regular PCI devices, which have a different layout.
> Does this make sense for some reason?

The first 16 bytes of the config space are common (which include
vendor/device). But some refactoring to have the header specific part
into separate functions would be nice?

Anthony: it did work for me, all 128 slots.

Re: [Qemu-devel] Re: questions on default_config_write in hw/pci.c

[M. Warner Losh]

In message: <20090430172955.GA16925@redhat.com>
            "Michael S. Tsirkin" <mst@redhat.com> writes:
: On Thu, Apr 30, 2009 at 02:15:26PM -0300, Marcelo Tosatti wrote:
: > On Thu, Apr 30, 2009 at 07:15:08PM +0300, Michael S. Tsirkin wrote:
: > > Hi,
: > > I've been looking at hw/pci.c, specifically at how config
: > > read/write are implemented, and have a couple of questions
: > > about default_config_write:
: > > 
: > > 1. The code at the beginning (if len == 4 ...)
: > >    seems to only update pci base registers if a dword write 
: > >    is performed. I think it's legal for the guest to perform 4
: > >    single-byte writes. Should this be supported?
: > > 
: > > 2. The large switch statement at the end of this function
: > >    uses hard-coded register offsets. Would it make sense
: > >    to change it to use macros from hw/pci.h?
: > > 
: > > 3. Still there I see:
: > >         switch(d->config[0x0e]) {
: > >         case 0x00:
: > >         case 0x80:
: > >    register 0x0e is header type, which has defined values
: > >    of 0x00 (device or host bridge), 0x01 (pci to pci bridge) and
: > >    0x02 (cardbus bridge). What is 0x80 and when is it used?
: > >    Would it make sense to remove this?
: > 
: > Don't know. Check the PCI spec?
: 
: That's the first thing I did :) What I site above is from there:
: "The Header Type field (located at offset 0Eh) defines what layout is
: provided. Currently three Header Types are defined, 00h which has the
: layout shown in Figure 6-1, 01h which is defined for PCI-to-PCI bridges
: and is documented in the PCI to PCI Bridge Architecture Specification,
: and 02h which is defined for CardBus bridges and is documented in the PC
: Card Standard."

Section 6.2.1:

Header Type

This byte identifies the layout of the second part of the predefined
header (beginning at byte 10h in Configuration Space) and also whether
or not the device contains multiple functions.  Bit 7 in this register
is used to identify multi-function device.  If the bit is 0, then the
device is single function.  If the bit is 1, then the device has
multiple functions.

: > > 4. Still there, there's some handling done for type 1 devices.
: > >    This support seems imcomplete.
: > >    Are there any PCI-to-PCI bridges emulated by qemu?
: > >    Would it make sense to remove this code?
: > 
: > 
: > It did work at one point:
: > 
: > http://www.mail-archive.com/kvm-devel@lists.sourceforge.net/msg16647.html
: 
: Sigh. I guess I'll have to fix it rather than just rip it out then ...
: What I refer to first of all is that writes to BAR registers go through
: logic for regular PCI devices, which have a different layout.
: Does this make sense for some reason?
: 
: -- 
: MST
: 
: 
: