[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions |
Date: |
Wed, 24 Aug 2011 15:39:29 +0300 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Wed, Aug 24, 2011 at 02:36:31PM +0200, Jan Kiszka wrote:
> On 2011-08-24 14:34, Michael S. Tsirkin wrote:
> > 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.
> > */
>
> I've expressed this in the commit message. Unless there is another
> reason to do v3, maybe you can merge the comment on commit.
>
> Jan
Sure, I can do that, no need with v3. You are fine with the way
it's formulated?
> --
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux
- Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions, (continued)
Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions, Michael S. Tsirkin, 2011/08/24
- Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions, Jan Kiszka, 2011/08/24
- Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions, Michael S. Tsirkin, 2011/08/24
- Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions, Michael S. Tsirkin, 2011/08/24
- Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions, Jan Kiszka, 2011/08/24
- Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions, Michael S. Tsirkin, 2011/08/24
- Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions, Jan Kiszka, 2011/08/24
- Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions,
Michael S. Tsirkin <=
- Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions, Jan Kiszka, 2011/08/24
[Qemu-devel] [PATCH v2] pci: Error on PCI capability collisions, Jan Kiszka, 2011/08/24