git: ba6e37e47f41 - main - ipmi_smbios: Deduplicate smbios entry point discovery logic

Cy Schubert Cy.Schubert at cschubert.com
Tue Feb 23 22:51:21 UTC 2021


In message <202102232119.11NLJw17099523 at gitrepo.freebsd.org>, Allan Jude 
writes
:
> The branch main has been updated by allanjude:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=ba6e37e47f41484fc61cc034619267b8
> 2ddd056c
>
> commit ba6e37e47f41484fc61cc034619267b82ddd056c
> Author:     Allan Jude <allanjude at FreeBSD.org>
> AuthorDate: 2021-02-23 21:17:37 +0000
> Commit:     Allan Jude <allanjude at FreeBSD.org>
> CommitDate: 2021-02-23 21:17:37 +0000
>
>     ipmi_smbios: Deduplicate smbios entry point discovery logic
>     
>     Sponsored by:   Ampere Computing LLC
>     Submitted by:   Klara Inc.
>     Reviewed by:    imp
>     Differential Revision:  https://reviews.freebsd.org/D28743
> ---
>  sys/dev/ipmi/ipmi_isa.c    |  1 +
>  sys/dev/ipmi/ipmi_pci.c    |  1 +
>  sys/dev/ipmi/ipmi_smbios.c | 46 ++++++++++++++++----------------------------
> --
>  sys/dev/ipmi/ipmi_smbus.c  |  1 +
>  sys/dev/smbios/smbios.c    | 21 +++++++++++++++++++++
>  sys/dev/smbios/smbios.h    |  2 ++
>  6 files changed, 42 insertions(+), 30 deletions(-)
>
> diff --git a/sys/dev/ipmi/ipmi_isa.c b/sys/dev/ipmi/ipmi_isa.c
> index cdff305b07ec..2831b53e4586 100644
> --- a/sys/dev/ipmi/ipmi_isa.c
> +++ b/sys/dev/ipmi/ipmi_isa.c
> @@ -286,3 +286,4 @@ static driver_t ipmi_isa_driver = {
>  };
>  
>  DRIVER_MODULE(ipmi_isa, isa, ipmi_isa_driver, ipmi_devclass, 0, 0);
> +MODULE_DEPEND(ipmi_isa, smbios, 1, 1, 1);
> diff --git a/sys/dev/ipmi/ipmi_pci.c b/sys/dev/ipmi/ipmi_pci.c
> index d4598f9db873..1697a4c31c2a 100644
> --- a/sys/dev/ipmi/ipmi_pci.c
> +++ b/sys/dev/ipmi/ipmi_pci.c
> @@ -179,6 +179,7 @@ static driver_t ipmi_pci_driver = {
>  };
>  
>  DRIVER_MODULE(ipmi_pci, pci, ipmi_pci_driver, ipmi_devclass, 0, 0);
> +MODULE_DEPEND(ipmi_pci, smbios, 1, 1, 1);
>  
>  /* Native IPMI on PCI driver. */
>  
> diff --git a/sys/dev/ipmi/ipmi_smbios.c b/sys/dev/ipmi/ipmi_smbios.c
> index 308a3b076ef7..735f404eec5f 100644
> --- a/sys/dev/ipmi/ipmi_smbios.c
> +++ b/sys/dev/ipmi/ipmi_smbios.c
> @@ -88,7 +88,7 @@ MTX_SYSINIT(ipmi_info, &ipmi_info_mtx, "ipmi info", MTX_DEF
> );
>  
>  static void	ipmi_smbios_probe(struct ipmi_get_info *);
>  static int	smbios_cksum(struct smbios_eps *);
> -static void	smbios_walk_table(uint8_t *, int, smbios_callback_t,
> +static void	smbios_walk_table(uint8_t *, vm_size_t, smbios_callback_t,
>  		    void *);
>  static void	smbios_ipmi_info(struct smbios_structure_header *, void *);
>  
> @@ -147,11 +147,12 @@ smbios_ipmi_info(struct smbios_structure_header *h, voi
> d *arg)
>  }
>  
>  static void
> -smbios_walk_table(uint8_t *p, int entries, smbios_callback_t cb, void *arg)
> +smbios_walk_table(uint8_t *table, vm_size_t size, smbios_callback_t cb, void
>  *arg)
>  {
>  	struct smbios_structure_header *s;
> +	uint8_t *p;
>  
> -	while (entries--) {
> +	for (p = table; p < table + size;) {
>  		s = (struct smbios_structure_header *)p;
>  		cb(s, arg);
>  
> @@ -160,8 +161,11 @@ smbios_walk_table(uint8_t *p, int entries, smbios_callba
> ck_t cb, void *arg)
>  		 * formatted area of this structure.
>  		 */
>  		p += s->length;
> -		while (!(p[0] == 0 && p[1] == 0))
> +		while (!(p[0] == 0 && p[1] == 0)) {
>  			p++;
> +			if (p >= table + size)
> +				return;
> +		}
>  
>  		/*
>  		 * Skip over the double-nul to the start of the next
> @@ -179,41 +183,23 @@ smbios_walk_table(uint8_t *p, int entries, smbios_callb
> ack_t cb, void *arg)
>  static void
>  ipmi_smbios_probe(struct ipmi_get_info *info)
>  {
> -	struct smbios_eps *header;
>  	void *table;
> -	u_int32_t addr;
> +	vm_paddr_t table_paddr;
> +	vm_size_t table_size;
> +	int err;
>  
>  	bzero(info, sizeof(struct ipmi_get_info));
>  
> -	/* Find the SMBIOS table header. */
> -	addr = bios_sigsearch(SMBIOS_START, SMBIOS_SIG, SMBIOS_LEN,
> -			      SMBIOS_STEP, SMBIOS_OFF);
> -	if (addr == 0)
> +	err = smbios_get_structure_table(&table_paddr, &table_size);
> +	if (err != 0)
>  		return;
>  
> -	/*
> -	 * Map the header.  We first map a fixed size to get the actual
> -	 * length and then map it a second time with the actual length so
> -	 * we can verify the checksum.
> -	 */
> -	header = pmap_mapbios(addr, sizeof(struct smbios_eps));
> -	table = pmap_mapbios(addr, header->length);
> -	pmap_unmapbios((vm_offset_t)header, sizeof(struct smbios_eps));
> -	header = table;
> -	if (smbios_cksum(header) != 0) {
> -		pmap_unmapbios((vm_offset_t)header, header->length);
> -		return;
> -	}
> +	table = pmap_mapbios(table_paddr, table_size);
>  
> -	/* Now map the actual table and walk it looking for an IPMI entry. */
> -	table = pmap_mapbios(header->structure_table_address,
> -	    header->structure_table_length);
> -	smbios_walk_table(table, header->number_structures, smbios_ipmi_info,
> -	    info);
> +	smbios_walk_table(table, table_size, smbios_ipmi_info, info);
>  
>  	/* Unmap everything. */
> -	pmap_unmapbios((vm_offset_t)table, header->structure_table_length);
> -	pmap_unmapbios((vm_offset_t)header, header->length);
> +	pmap_unmapbios((vm_offset_t)table, table_size);
>  }
>  
>  /*
> diff --git a/sys/dev/ipmi/ipmi_smbus.c b/sys/dev/ipmi/ipmi_smbus.c
> index 212248f0217e..bc7377e5aee8 100644
> --- a/sys/dev/ipmi/ipmi_smbus.c
> +++ b/sys/dev/ipmi/ipmi_smbus.c
> @@ -131,3 +131,4 @@ static driver_t ipmi_smbus_driver = {
>  
>  DRIVER_MODULE(ipmi_smbus, smbus, ipmi_smbus_driver, ipmi_devclass, 0, 0);
>  MODULE_DEPEND(ipmi_smbus, smbus, SMBUS_MINVER, SMBUS_PREFVER, SMBUS_MAXVER);
> +MODULE_DEPEND(ipmi_smbus, smbios, 1, 1, 1);
> diff --git a/sys/dev/smbios/smbios.c b/sys/dev/smbios/smbios.c
> index 10589ed8d49d..00c7e2c98d2a 100644
> --- a/sys/dev/smbios/smbios.c
> +++ b/sys/dev/smbios/smbios.c
> @@ -51,6 +51,8 @@ __FBSDID("$FreeBSD$");
>  #endif
>  #include <dev/smbios/smbios.h>
>  
> +static struct smbios_softc *smbios;
> +
>  /*
>   * System Management BIOS Reference Specification, v2.4 Final
>   * http://www.dmtf.org/standards/published_documents/DSP0134.pdf
> @@ -179,6 +181,7 @@ smbios_attach (device_t dev)
>  			bcd2bin(sc->eps->BCD_revision & 0x0f));
>  	printf("\n");
>  
> +	smbios = sc;
>  	return (0);
>  bad:
>  	if (sc->res)
> @@ -191,6 +194,7 @@ smbios_detach (device_t dev)
>  {
>  	struct smbios_softc *sc;
>  
> +	smbios = NULL;
>  	sc = device_get_softc(dev);
>  
>  	if (sc->res)
> @@ -199,6 +203,23 @@ smbios_detach (device_t dev)
>  	return (0);
>  }
>  
> +int
> +smbios_get_structure_table(vm_paddr_t *table, vm_size_t *size)
> +{
> +
> +	if (smbios == NULL)
> +		return (ENXIO);
> +	if (smbios->eps_64bit) {
> +		*table = smbios->eps3->structure_table_address;
> +		*size = smbios->eps3->structure_table_max_size;

This had some undesired results.

--- all_subdir_bios/smbios ---
/opt/src/git-src/sys/dev/smbios/smbios.c:212:14: error: no member named 
'eps_64bit' in 'struct smbios_softc'
        if (smbios->eps_64bit) {
            ~~~~~~  ^
/opt/src/git-src/sys/dev/smbios/smbios.c:213:20: error: no member named 
'eps3' in 'struct smbios_softc'; did you mean 'eps'?
                *table = smbios->eps3->structure_table_address;
                                 ^~~~
                                 eps
/opt/src/git-src/sys/dev/smbios/smbios.c:66:22: note: 'eps' declared here
        struct smbios_eps *     eps;
                                ^
/opt/src/git-src/sys/dev/smbios/smbios.c:214:19: error: no member named 
'eps3' in 'struct smbios_softc'; did you mean 'eps'?
                *size = smbios->eps3->structure_table_max_size;
                                ^~~~
                                eps
/opt/src/git-src/sys/dev/smbios/smbios.c:66:22: note: 'eps' declared here
        struct smbios_eps *     eps;
                                ^
/opt/src/git-src/sys/dev/smbios/smbios.c:214:25: error: no member named 
'structure_table_max_size' in 'struct smbios_eps'; did you mean 
'structure_table_address'?
                *size = smbios->eps3->structure_table_max_size;
                                      ^~~~~~~~~~~~~~~~~~~~~~~~
                                      structure_table_address
/opt/src/git-src/sys/dev/smbios/smbios.h:58:11: note: 
'structure_table_address' declared here
        uint32_t        structure_table_address;
                        ^


> +	} else {
> +		*table = smbios->eps->structure_table_address;
> +		*size = smbios->eps->structure_table_length;
> +	}
> +	return (0);
> +}
> +
> +
>  static int
>  smbios_modevent (mod, what, arg)
>          module_t        mod;
> diff --git a/sys/dev/smbios/smbios.h b/sys/dev/smbios/smbios.h
> index 6503cdb73c4c..554b882f1da4 100644
> --- a/sys/dev/smbios/smbios.h
> +++ b/sys/dev/smbios/smbios.h
> @@ -32,6 +32,8 @@
>  #ifndef _SMBIOS_H_
>  #define _SMBIOS_H_
>  
> +int smbios_get_structure_table(vm_paddr_t *table, vm_size_t *size);
> +
>  /*
>   * System Management BIOS
>   */
>



-- 
Cheers,
Cy Schubert <Cy.Schubert at cschubert.com>
FreeBSD UNIX:  <cy at FreeBSD.org>   Web:  https://FreeBSD.org
NTP:           <cy at nwtime.org>    Web:  https://nwtime.org

	The need of the many outweighs the greed of the few.





More information about the dev-commits-src-all mailing list