Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1294070 > unrolled thread

Re: [PATCHv5 7/7] pciutils: Allow 32-bit domains

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCHv5 7/7] pciutils: Allow 32-bit domains Bjorn Helgaas <helgaas@kernel.org> - 2015-12-17 18:20 +0100
    Re: [PATCHv5 7/7] pciutils: Allow 32-bit domains Keith Busch <keith.busch@intel.com> - 2015-12-17 18:40 +0100
      Re: [PATCHv5 7/7] pciutils: Allow 32-bit domains Bjorn Helgaas <helgaas@kernel.org> - 2015-12-17 19:30 +0100

#1294070 — Re: [PATCHv5 7/7] pciutils: Allow 32-bit domains

Hi Keith,

On Mon, Dec 07, 2015 at 02:32:29PM -0700, Keith Busch wrote:
> PCI-e segments will continue to use the lower 16 bits as required by
> ACPI. Special domains may use the full 32-bits.
> 
> Signed-off-by: Keith Busch <keith.busch@intel.com>
> ---
>  lib/filter.c |    2 +-
>  lib/pci.h    |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/filter.c b/lib/filter.c
> index d4254a0..075dc2f 100644
> --- a/lib/filter.c
> +++ b/lib/filter.c
> @@ -45,7 +45,7 @@ pci_filter_parse_slot_v33(struct pci_filter *f, char *str)
>  	  if (str[0] && strcmp(str, "*"))
>  	    {
>  	      long int x = strtol(str, &e, 16);
> -	      if ((e && *e) || (x < 0 || x > 0xffff))
> +	      if ((e && *e) || (x < 0))

Just out of curiosity (I don't maintain pciutils; Martin would apply
this one), is there some part of the PCI or PCI firmware spec that is
relevant to this change?  Maybe this is connected to parsing things
exported by the kernel and not directly tied to PCI at the spec level.

Whatever it is, a pointer to the producer of the information you're
consuming here would help us understand and review the patch.

>  		return "Invalid domain number";
>  	      f->domain = x;
>  	    }
> diff --git a/lib/pci.h b/lib/pci.h
> index 10ba831..7e42765 100644
> --- a/lib/pci.h
> +++ b/lib/pci.h
> @@ -119,7 +119,7 @@ struct pci_param *pci_walk_params(struct pci_access *acc, struct pci_param *prev
>  
>  struct pci_dev {
>    struct pci_dev *next;			/* Next device in the chain */
> -  u16 domain;				/* PCI domain (host bridge) */
> +  int32_t domain;			/* PCI domain (host bridge) */
>    u8 bus, dev, func;			/* Bus inside domain, device and function */
>  
>    /* These fields are set by pci_fill_info() */
> -- 
> 1.7.10.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1294078

On Thu, Dec 17, 2015 at 11:15:45AM -0600, Bjorn Helgaas wrote:
> > @@ -45,7 +45,7 @@ pci_filter_parse_slot_v33(struct pci_filter *f, char *str)
> >  	  if (str[0] && strcmp(str, "*"))
> >  	    {
> >  	      long int x = strtol(str, &e, 16);
> > -	      if ((e && *e) || (x < 0 || x > 0xffff))
> > +	      if ((e && *e) || (x < 0))
> 
> Just out of curiosity (I don't maintain pciutils; Martin would apply
> this one), is there some part of the PCI or PCI firmware spec that is
> relevant to this change?  Maybe this is connected to parsing things
> exported by the kernel and not directly tied to PCI at the spec level.
>
> Whatever it is, a pointer to the producer of the information you're
> consuming here would help us understand and review the patch.

Hi Bjorn,

This is not tied to anything defined in PCI spec. Domain numbers being
a software construct (ACPI6, §6.5.6), we don't need to constrain the
representation. ACPI defines 16-bit segments, and domains provided by
this new host bridge do not define _SEG, so this series proposes domain
numbers outside the ACPI reachable range to avoid potential clashes.

The pciutils patch just synchronizes the essential tooling software with
the kernel software's new representation.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1294122

On Thu, Dec 17, 2015 at 05:34:46PM +0000, Keith Busch wrote:
> On Thu, Dec 17, 2015 at 11:15:45AM -0600, Bjorn Helgaas wrote:
> > > @@ -45,7 +45,7 @@ pci_filter_parse_slot_v33(struct pci_filter *f, char *str)
> > >  	  if (str[0] && strcmp(str, "*"))
> > >  	    {
> > >  	      long int x = strtol(str, &e, 16);
> > > -	      if ((e && *e) || (x < 0 || x > 0xffff))
> > > +	      if ((e && *e) || (x < 0))
> > 
> > Just out of curiosity (I don't maintain pciutils; Martin would apply
> > this one), is there some part of the PCI or PCI firmware spec that is
> > relevant to this change?  Maybe this is connected to parsing things
> > exported by the kernel and not directly tied to PCI at the spec level.
> >
> > Whatever it is, a pointer to the producer of the information you're
> > consuming here would help us understand and review the patch.
> 
> Hi Bjorn,
> 
> This is not tied to anything defined in PCI spec. Domain numbers being
> a software construct (ACPI6, §6.5.6), we don't need to constrain the
> representation. ACPI defines 16-bit segments, and domains provided by
> this new host bridge do not define _SEG, so this series proposes domain
> numbers outside the ACPI reachable range to avoid potential clashes.
> 
> The pciutils patch just synchronizes the essential tooling software with
> the kernel software's new representation.

That's what I figured.  It'd be useful to know exactly what is on the
other end of this, e.g., a Linux /proc or /sys file or whatever it is.

Your changelog assumes a lot of implicit knowledge about Linux, VMD,
and the previous patches in this series.  But pciutils is not
Linux-specific, and it's maintained completely separately from Linux.

This patch needs to supply enough explicit context that it makes sense
all by itself, apart from the kernel series.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web