Jan Beulich
2011-Jun-21 08:01 UTC
[Xen-devel] [PATCH] x86/DMI: use proper structures in favor of byte offsets
Besides being (in my eyes) desirable cleanup, this at once represents another prerequisite for native EFI booting support. Signed-off-by: Jan Beulich <jbeulich@novell.com> --- a/xen/arch/x86/dmi_scan.c +++ b/xen/arch/x86/dmi_scan.c @@ -10,11 +10,31 @@ #include <asm/system.h> #include <xen/dmi.h> -#define bt_ioremap(b,l) ((u8 *)__acpi_map_table(b,l)) +#define bt_ioremap(b,l) ((void *)__acpi_map_table(b,l)) #define bt_iounmap(b,l) ((void)0) #define memcpy_fromio memcpy #define alloc_bootmem(l) xmalloc_bytes(l) +struct dmi_eps { + char anchor[5]; /* "_DMI_" */ + u8 checksum; + u16 size; + u32 address; + u16 num_structures; + u8 revision; +} __attribute__((packed)); + +struct smbios_eps { + char anchor[4]; /* "_SM_" */ + u8 checksum; + u8 length; + u8 major, minor; + u16 max_size; + u8 revision; + u8 _rsrvd_[5]; + struct dmi_eps dmi; +} __attribute__((packed)); + struct dmi_header { u8 type; @@ -90,62 +110,70 @@ static int __init dmi_table(u32 base, in } -inline static int __init dmi_checksum(u8 *buf) +static inline bool_t __init dmi_checksum(const void __iomem *buf, + unsigned int len) { - u8 sum=0; - int a; + u8 sum = 0; + const u8 *p = buf; + unsigned int a; - for(a=0; a<15; a++) - sum+=buf[a]; - return (sum==0); + for (a = 0; a < len; a++) + sum += p[a]; + return sum == 0; } int __init dmi_get_table(u32 *base, u32 *len) { - u8 buf[15]; + struct dmi_eps eps; char __iomem *p, *q; p = maddr_to_virt(0xF0000); for (q = p; q < p + 0x10000; q += 16) { - memcpy_fromio(buf, q, 15); - if (memcmp(buf, "_DMI_", 5)==0 && dmi_checksum(buf)) { - *base=buf[11]<<24|buf[10]<<16|buf[9]<<8|buf[8]; - *len=buf[7]<<8|buf[6]; + memcpy_fromio(&eps, q, 15); + if (memcmp(eps.anchor, "_DMI_", 5) == 0 && + dmi_checksum(&eps, sizeof(eps))) { + *base = eps.address; + *len = eps.size; return 0; } } return -1; } +static int __init _dmi_iterate(const struct dmi_eps *dmi, + const struct smbios_eps __iomem *smbios, + void (*decode)(struct dmi_header *)) +{ + u16 num = dmi->num_structures; + u16 len = dmi->size; + u32 base = dmi->address; + + /* + * DMI version 0.0 means that the real version is taken from + * the SMBIOS version, which we may not know at this point. + */ + if (dmi->revision) + printk(KERN_INFO "DMI %d.%d present.\n", + dmi->revision >> 4, dmi->revision & 0x0f); + else if (!smbios) + printk(KERN_INFO "DMI present.\n"); + dmi_printk((KERN_INFO "%d structures occupying %d bytes.\n", + num, len)); + dmi_printk((KERN_INFO "DMI table at 0x%08X.\n", base)); + return dmi_table(base, len, num, decode); +} + static int __init dmi_iterate(void (*decode)(struct dmi_header *)) { - u8 buf[15]; + struct dmi_eps eps; char __iomem *p, *q; p = maddr_to_virt(0xF0000); for (q = p; q < p + 0x10000; q += 16) { - memcpy_fromio(buf, q, 15); - if (memcmp(buf, "_DMI_", 5)==0 && dmi_checksum(buf)) { - u16 num=buf[13]<<8|buf[12]; - u16 len=buf[7]<<8|buf[6]; - u32 base=buf[11]<<24|buf[10]<<16|buf[9]<<8|buf[8]; - - /* - * DMI version 0.0 means that the real version is taken from - * the SMBIOS version, which we don''t know at this point. - */ - if(buf[14]!=0) - printk(KERN_INFO "DMI %d.%d present.\n", - buf[14]>>4, buf[14]&0x0F); - else - printk(KERN_INFO "DMI present.\n"); - dmi_printk((KERN_INFO "%d structures occupying %d bytes.\n", - num, len)); - dmi_printk((KERN_INFO "DMI table at 0x%08X.\n", - base)); - if(dmi_table(base,len, num, decode)==0) - return 0; - } + memcpy_fromio(&eps, q, sizeof(eps)); + if (memcmp(eps.anchor, "_DMI_", 5) == 0 && + dmi_checksum(&eps, sizeof(eps))) + return _dmi_iterate(&eps, NULL, decode); } return -1; } _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Chris Dalton
2011-Jun-21 09:48 UTC
Re: [Xen-devel] [PATCH] x86/DMI: use proper structures in favor of byte offsets
Hi Jan, Not a comment on the patch changes directly but on the code covered by the patch: in my experience on some EFI bioses you can''t assume that the SMBIOS tables are in a region beginning 0xF0000. The location needs to be retrieved from the EFI system table, but I guess you have this in hand. Chris On 21/06/11 09:01, Jan Beulich wrote:> Besides being (in my eyes) desirable cleanup, this at once represents > another prerequisite for native EFI booting support. > > Signed-off-by: Jan Beulich<jbeulich@novell.com> > > --- a/xen/arch/x86/dmi_scan.c > +++ b/xen/arch/x86/dmi_scan.c > @@ -10,11 +10,31 @@ > #include<asm/system.h> > #include<xen/dmi.h> > > -#define bt_ioremap(b,l) ((u8 *)__acpi_map_table(b,l)) > +#define bt_ioremap(b,l) ((void *)__acpi_map_table(b,l)) > #define bt_iounmap(b,l) ((void)0) > #define memcpy_fromio memcpy > #define alloc_bootmem(l) xmalloc_bytes(l) > > +struct dmi_eps { > + char anchor[5]; /* "_DMI_" */ > + u8 checksum; > + u16 size; > + u32 address; > + u16 num_structures; > + u8 revision; > +} __attribute__((packed)); > + > +struct smbios_eps { > + char anchor[4]; /* "_SM_" */ > + u8 checksum; > + u8 length; > + u8 major, minor; > + u16 max_size; > + u8 revision; > + u8 _rsrvd_[5]; > + struct dmi_eps dmi; > +} __attribute__((packed)); > + > struct dmi_header > { > u8 type; > @@ -90,62 +110,70 @@ static int __init dmi_table(u32 base, in > } > > > -inline static int __init dmi_checksum(u8 *buf) > +static inline bool_t __init dmi_checksum(const void __iomem *buf, > + unsigned int len) > { > - u8 sum=0; > - int a; > + u8 sum = 0; > + const u8 *p = buf; > + unsigned int a; > > - for(a=0; a<15; a++) > - sum+=buf[a]; > - return (sum==0); > + for (a = 0; a< len; a++) > + sum += p[a]; > + return sum == 0; > } > > int __init dmi_get_table(u32 *base, u32 *len) > { > - u8 buf[15]; > + struct dmi_eps eps; > char __iomem *p, *q; > > p = maddr_to_virt(0xF0000); > for (q = p; q< p + 0x10000; q += 16) { > - memcpy_fromio(buf, q, 15); > - if (memcmp(buf, "_DMI_", 5)==0&& dmi_checksum(buf)) { > - *base=buf[11]<<24|buf[10]<<16|buf[9]<<8|buf[8]; > - *len=buf[7]<<8|buf[6]; > + memcpy_fromio(&eps, q, 15); > + if (memcmp(eps.anchor, "_DMI_", 5) == 0&& > + dmi_checksum(&eps, sizeof(eps))) { > + *base = eps.address; > + *len = eps.size; > return 0; > } > } > return -1; > } > > +static int __init _dmi_iterate(const struct dmi_eps *dmi, > + const struct smbios_eps __iomem *smbios, > + void (*decode)(struct dmi_header *)) > +{ > + u16 num = dmi->num_structures; > + u16 len = dmi->size; > + u32 base = dmi->address; > + > + /* > + * DMI version 0.0 means that the real version is taken from > + * the SMBIOS version, which we may not know at this point. > + */ > + if (dmi->revision) > + printk(KERN_INFO "DMI %d.%d present.\n", > + dmi->revision>> 4, dmi->revision& 0x0f); > + else if (!smbios) > + printk(KERN_INFO "DMI present.\n"); > + dmi_printk((KERN_INFO "%d structures occupying %d bytes.\n", > + num, len)); > + dmi_printk((KERN_INFO "DMI table at 0x%08X.\n", base)); > + return dmi_table(base, len, num, decode); > +} > + > static int __init dmi_iterate(void (*decode)(struct dmi_header *)) > { > - u8 buf[15]; > + struct dmi_eps eps; > char __iomem *p, *q; > > p = maddr_to_virt(0xF0000); > for (q = p; q< p + 0x10000; q += 16) { > - memcpy_fromio(buf, q, 15); > - if (memcmp(buf, "_DMI_", 5)==0&& dmi_checksum(buf)) { > - u16 num=buf[13]<<8|buf[12]; > - u16 len=buf[7]<<8|buf[6]; > - u32 base=buf[11]<<24|buf[10]<<16|buf[9]<<8|buf[8]; > - > - /* > - * DMI version 0.0 means that the real version is taken from > - * the SMBIOS version, which we don''t know at this point. > - */ > - if(buf[14]!=0) > - printk(KERN_INFO "DMI %d.%d present.\n", > - buf[14]>>4, buf[14]&0x0F); > - else > - printk(KERN_INFO "DMI present.\n"); > - dmi_printk((KERN_INFO "%d structures occupying %d bytes.\n", > - num, len)); > - dmi_printk((KERN_INFO "DMI table at 0x%08X.\n", > - base)); > - if(dmi_table(base,len, num, decode)==0) > - return 0; > - } > + memcpy_fromio(&eps, q, sizeof(eps)); > + if (memcmp(eps.anchor, "_DMI_", 5) == 0&& > + dmi_checksum(&eps, sizeof(eps))) > + return _dmi_iterate(&eps, NULL, decode); > } > return -1; > } > > > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel-- +44(0)7827 233109 Registered No: 690597 (Hewlett-Packard Ltd) Registered Office: Cain Rd, Bracknell, Berks, RG12 1HN, UK _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2011-Jun-21 10:02 UTC
Re: [Xen-devel] [PATCH] x86/DMI: use proper structures in favor of byte offsets
>>> On 21.06.11 at 11:48, Chris Dalton <cid@hp.com> wrote: > Not a comment on the patch changes directly but on the code covered by > the patch: in my experience on some EFI bioses you can''t assume that the > SMBIOS tables are in a region beginning 0xF0000. The location needs to > be retrieved from the EFI system table, but I guess you have this in hand.Yes, of course. The change here only is to not add further byte-offset hacks in the code that''s going to be added with the EFI patches I''m preparing. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel