Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions

[Michael S. Tsirkin]

On Wed, Aug 24, 2011 at 02:29:36PM +0200, Jan Kiszka wrote:
> On 2011-08-24 13:58, Michael S. Tsirkin wrote:
> > On Wed, Aug 24, 2011 at 12:10:32PM +0200, Jan Kiszka wrote:
> >> On 2011-08-24 12:04, Michael S. Tsirkin wrote:
> >>> On Tue, Aug 23, 2011 at 07:28:08PM +0200, Jan Kiszka wrote:
> >>>> From: Alex Williamson <address@hidden>
> >>>>
> >>>> Nothing good can happen when we overlap capabilities
> >>>>
> >>>> [ Jan: rebased over qemu, minor formatting ]
> >>>>
> >>>> Signed-off-by: Jan Kiszka <address@hidden>
> >>>
> >>> This doesn't build for me:
> >>>
> >>> /scm/qemu/hw/pci.c: In function ‘pci_add_capability’:
> >>> /scm/qemu/hw/pci.c:1970:45: error: ‘PCIDevice’ has no member named 
> >>> ‘config_map’
> >>
> >> Yeah, sorry, forgot to refresh the commit before posting.
> >>
> >>>
> >>> I think that what that includes is the capability including each given
> >>> offset, right?  It would be easy to write some code scanning the
> >>> capability list to figure this value out.
> >>> Something along the lines of (untested):
> >>>
> >>> static
> >>> uint8_t pci_find_capability_at_offset(PCIDevice *pdev, uint8_t offset)
> >>> {                                       
> >>>     uint8_t next, prev, found = 0;
> >>>
> >>>     if (!(pdev->config[PCI_STATUS] & PCI_STATUS_CAP_LIST))
> >>>         return 0;
> >>>
> >>>     for (prev = PCI_CAPABILITY_LIST; (next = pdev->config[prev]);
> >>>          prev = next + PCI_CAP_LIST_NEXT)
> >>>         if (next <= offset && next > found)
> >>>             found = next;
> >>>
> >>>     return found;
> >>> }
> >>
> >> Sounds useful, will enhance the patch.
> >>
> >> (Originally, I just wanted to reduce the qemu-kvm delta... :) )
> >>
> >> Jan
> > 
> > Also, let's add a comment documenting the
> > reason for this check: device assignment
> > depends on this check to verify that the device
> > is not broken.
> 
> Based on the previous discussion, I don't think this is accurate as it
> will also validate emulated devices.
> 
> Jan

Something like the below is accurate, right?

/* Device assignment depends on this check to verify that the device
   is not broken. Should never trigger for emulated devices,
   but it's helpful for debugging these.
 */


> -- 
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux