Message ID | 1478502595-8484-3-git-send-email-jasowang@redhat.com |
---|---|
State | New |
Headers | show |
On Mon, Nov 07, 2016 at 03:09:47PM +0800, Jason Wang wrote: > To avoid duplicated name and ease debugging. > > Cc: Michael S. Tsirkin <mst@redhat.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Richard Henderson <rth@twiddle.net> > Cc: Eduardo Habkost <ehabkost@redhat.com> > Acked-by: Peter Xu <peterx@redhat.com> > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > hw/i386/intel_iommu.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > index 1655a65..5272c30 100644 > --- a/hw/i386/intel_iommu.c > +++ b/hw/i386/intel_iommu.c > @@ -2323,6 +2323,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn) > uintptr_t key = (uintptr_t)bus; > VTDBus *vtd_bus = g_hash_table_lookup(s->vtd_as_by_busptr, &key); > VTDAddressSpace *vtd_dev_as; > + char name[128]; > > if (!vtd_bus) { > /* No corresponding free() */ > @@ -2336,6 +2337,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn) > vtd_dev_as = vtd_bus->dev_as[devfn]; > > if (!vtd_dev_as) { > + snprintf(name, sizeof(name), "intel_iommu_devfn_%d", devfn); Better with bus number as well? ;) Like: "%02x:%02x.%x" for (bus, dev, fn). Thanks, -- peterx
On 2016年12月16日 10:53, Peter Xu wrote: > On Mon, Nov 07, 2016 at 03:09:47PM +0800, Jason Wang wrote: >> To avoid duplicated name and ease debugging. >> >> Cc: Michael S. Tsirkin <mst@redhat.com> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Cc: Richard Henderson <rth@twiddle.net> >> Cc: Eduardo Habkost <ehabkost@redhat.com> >> Acked-by: Peter Xu <peterx@redhat.com> >> Signed-off-by: Jason Wang <jasowang@redhat.com> >> --- >> hw/i386/intel_iommu.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c >> index 1655a65..5272c30 100644 >> --- a/hw/i386/intel_iommu.c >> +++ b/hw/i386/intel_iommu.c >> @@ -2323,6 +2323,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn) >> uintptr_t key = (uintptr_t)bus; >> VTDBus *vtd_bus = g_hash_table_lookup(s->vtd_as_by_busptr, &key); >> VTDAddressSpace *vtd_dev_as; >> + char name[128]; >> >> if (!vtd_bus) { >> /* No corresponding free() */ >> @@ -2336,6 +2337,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn) >> vtd_dev_as = vtd_bus->dev_as[devfn]; >> >> if (!vtd_dev_as) { >> + snprintf(name, sizeof(name), "intel_iommu_devfn_%d", devfn); > Better with bus number as well? ;) > > Like: "%02x:%02x.%x" for (bus, dev, fn). > > Thanks, > > -- peterx Yes, will do it in next version. Thanks
On 2016年12月16日 11:53, Jason Wang wrote: > > > On 2016年12月16日 10:53, Peter Xu wrote: >> On Mon, Nov 07, 2016 at 03:09:47PM +0800, Jason Wang wrote: >>> To avoid duplicated name and ease debugging. >>> >>> Cc: Michael S. Tsirkin <mst@redhat.com> >>> Cc: Paolo Bonzini <pbonzini@redhat.com> >>> Cc: Richard Henderson <rth@twiddle.net> >>> Cc: Eduardo Habkost <ehabkost@redhat.com> >>> Acked-by: Peter Xu <peterx@redhat.com> >>> Signed-off-by: Jason Wang <jasowang@redhat.com> >>> --- >>> hw/i386/intel_iommu.c | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c >>> index 1655a65..5272c30 100644 >>> --- a/hw/i386/intel_iommu.c >>> +++ b/hw/i386/intel_iommu.c >>> @@ -2323,6 +2323,7 @@ VTDAddressSpace >>> *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn) >>> uintptr_t key = (uintptr_t)bus; >>> VTDBus *vtd_bus = g_hash_table_lookup(s->vtd_as_by_busptr, &key); >>> VTDAddressSpace *vtd_dev_as; >>> + char name[128]; >>> if (!vtd_bus) { >>> /* No corresponding free() */ >>> @@ -2336,6 +2337,7 @@ VTDAddressSpace >>> *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn) >>> vtd_dev_as = vtd_bus->dev_as[devfn]; >>> if (!vtd_dev_as) { >>> + snprintf(name, sizeof(name), "intel_iommu_devfn_%d", devfn); >> Better with bus number as well? ;) >> >> Like: "%02x:%02x.%x" for (bus, dev, fn). >> >> Thanks, >> >> -- peterx > > Yes, will do it in next version. > > Thanks > Actually, I choose not to bother since bus and fn number needs more codes to get. Thanks
On Fri, Dec 30, 2016 at 04:19:31PM +0800, Jason Wang wrote: > > > On 2016年12月16日 11:53, Jason Wang wrote: > > > > > >On 2016年12月16日 10:53, Peter Xu wrote: > >>On Mon, Nov 07, 2016 at 03:09:47PM +0800, Jason Wang wrote: > >>>To avoid duplicated name and ease debugging. > >>> > >>>Cc: Michael S. Tsirkin <mst@redhat.com> > >>>Cc: Paolo Bonzini <pbonzini@redhat.com> > >>>Cc: Richard Henderson <rth@twiddle.net> > >>>Cc: Eduardo Habkost <ehabkost@redhat.com> > >>>Acked-by: Peter Xu <peterx@redhat.com> > >>>Signed-off-by: Jason Wang <jasowang@redhat.com> > >>>--- > >>> hw/i386/intel_iommu.c | 4 +++- > >>> 1 file changed, 3 insertions(+), 1 deletion(-) > >>> > >>>diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > >>>index 1655a65..5272c30 100644 > >>>--- a/hw/i386/intel_iommu.c > >>>+++ b/hw/i386/intel_iommu.c > >>>@@ -2323,6 +2323,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState > >>>*s, PCIBus *bus, int devfn) > >>> uintptr_t key = (uintptr_t)bus; > >>> VTDBus *vtd_bus = g_hash_table_lookup(s->vtd_as_by_busptr, &key); > >>> VTDAddressSpace *vtd_dev_as; > >>>+ char name[128]; > >>> if (!vtd_bus) { > >>> /* No corresponding free() */ > >>>@@ -2336,6 +2337,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState > >>>*s, PCIBus *bus, int devfn) > >>> vtd_dev_as = vtd_bus->dev_as[devfn]; > >>> if (!vtd_dev_as) { > >>>+ snprintf(name, sizeof(name), "intel_iommu_devfn_%d", devfn); > >>Better with bus number as well? ;) > >> > >>Like: "%02x:%02x.%x" for (bus, dev, fn). > >> > >>Thanks, > >> > >>-- peterx > > > >Yes, will do it in next version. > > > >Thanks > > > > Actually, I choose not to bother since bus and fn number needs more codes to > get. Should this work? snprintf(name, sizeof(name), "intel_iommu_%02x:%02x.%x", pci_bus_num(bus), VTD_PCI_SLOT(devfn), VTD_PCI_FUNC(devfn)); Anyway, I am okay as well to keep it as it is. :) -- peterx
On 2016年12月30日 17:22, Peter Xu wrote: > On Fri, Dec 30, 2016 at 04:19:31PM +0800, Jason Wang wrote: >> >> On 2016年12月16日 11:53, Jason Wang wrote: >>> >>> On 2016年12月16日 10:53, Peter Xu wrote: >>>> On Mon, Nov 07, 2016 at 03:09:47PM +0800, Jason Wang wrote: >>>>> To avoid duplicated name and ease debugging. >>>>> >>>>> Cc: Michael S. Tsirkin <mst@redhat.com> >>>>> Cc: Paolo Bonzini <pbonzini@redhat.com> >>>>> Cc: Richard Henderson <rth@twiddle.net> >>>>> Cc: Eduardo Habkost <ehabkost@redhat.com> >>>>> Acked-by: Peter Xu <peterx@redhat.com> >>>>> Signed-off-by: Jason Wang <jasowang@redhat.com> >>>>> --- >>>>> hw/i386/intel_iommu.c | 4 +++- >>>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c >>>>> index 1655a65..5272c30 100644 >>>>> --- a/hw/i386/intel_iommu.c >>>>> +++ b/hw/i386/intel_iommu.c >>>>> @@ -2323,6 +2323,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState >>>>> *s, PCIBus *bus, int devfn) >>>>> uintptr_t key = (uintptr_t)bus; >>>>> VTDBus *vtd_bus = g_hash_table_lookup(s->vtd_as_by_busptr, &key); >>>>> VTDAddressSpace *vtd_dev_as; >>>>> + char name[128]; >>>>> if (!vtd_bus) { >>>>> /* No corresponding free() */ >>>>> @@ -2336,6 +2337,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState >>>>> *s, PCIBus *bus, int devfn) >>>>> vtd_dev_as = vtd_bus->dev_as[devfn]; >>>>> if (!vtd_dev_as) { >>>>> + snprintf(name, sizeof(name), "intel_iommu_devfn_%d", devfn); >>>> Better with bus number as well? ;) >>>> >>>> Like: "%02x:%02x.%x" for (bus, dev, fn). >>>> >>>> Thanks, >>>> >>>> -- peterx >>> Yes, will do it in next version. >>> >>> Thanks >>> >> Actually, I choose not to bother since bus and fn number needs more codes to >> get. > Should this work? > > snprintf(name, sizeof(name), "intel_iommu_%02x:%02x.%x", > pci_bus_num(bus), VTD_PCI_SLOT(devfn), > VTD_PCI_FUNC(devfn)); > > Anyway, I am okay as well to keep it as it is. :) > > -- peterx This may work but I tend to keep it as is and make this on top :) Thanks
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 1655a65..5272c30 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -2323,6 +2323,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn) uintptr_t key = (uintptr_t)bus; VTDBus *vtd_bus = g_hash_table_lookup(s->vtd_as_by_busptr, &key); VTDAddressSpace *vtd_dev_as; + char name[128]; if (!vtd_bus) { /* No corresponding free() */ @@ -2336,6 +2337,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn) vtd_dev_as = vtd_bus->dev_as[devfn]; if (!vtd_dev_as) { + snprintf(name, sizeof(name), "intel_iommu_devfn_%d", devfn); vtd_bus->dev_as[devfn] = vtd_dev_as = g_malloc0(sizeof(VTDAddressSpace)); vtd_dev_as->bus = bus; @@ -2350,7 +2352,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn) memory_region_add_subregion(&vtd_dev_as->iommu, VTD_INTERRUPT_ADDR_FIRST, &vtd_dev_as->iommu_ir); address_space_init(&vtd_dev_as->as, - &vtd_dev_as->iommu, "intel_iommu"); + &vtd_dev_as->iommu, name); } return vtd_dev_as; }