| Message ID | 1420774255-702-3-git-send-email-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Simon, On Fri, Jan 9, 2015 at 11:30 AM, Simon Glass <sjg@chromium.org> wrote: > The existing IP checksum function is only accessible to the 'coreboot' cpu. > Move it into the common area and rename it slightly to remove the > abbreviations. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > Changes in v2: > - Refactor IP checksum patches > > arch/x86/cpu/Makefile | 1 + > arch/x86/cpu/coreboot/Makefile | 1 - > arch/x86/cpu/coreboot/ipchecksum.c | 55 ------------------------- > arch/x86/cpu/coreboot/tables.c | 8 ++-- > arch/x86/cpu/ip_checksum.c | 34 +++++++++++++++ What about the namings we discussed in the v1 patch thread? ip_xxx indicates ip protocol, but acutally they are not. [snip] Regards, Bin
Hi Bin, On 10 January 2015 at 08:47, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Fri, Jan 9, 2015 at 11:30 AM, Simon Glass <sjg@chromium.org> wrote: >> The existing IP checksum function is only accessible to the 'coreboot' cpu. >> Move it into the common area and rename it slightly to remove the >> abbreviations. >> >> Signed-off-by: Simon Glass <sjg@chromium.org> >> --- >> >> Changes in v2: >> - Refactor IP checksum patches >> >> arch/x86/cpu/Makefile | 1 + >> arch/x86/cpu/coreboot/Makefile | 1 - >> arch/x86/cpu/coreboot/ipchecksum.c | 55 ------------------------- >> arch/x86/cpu/coreboot/tables.c | 8 ++-- >> arch/x86/cpu/ip_checksum.c | 34 +++++++++++++++ > > What about the namings we discussed in the v1 patch thread? ip_xxx > indicates ip protocol, but acutally they are not. Is it not? This is not actually CMOS-specific - e.g. it is used by Coreboot to send data through to U-Boot in the tables it provides. I thought it was an IP checksum... Regards, Simon
Hi Simon, On Sun, Jan 11, 2015 at 1:20 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Bin, > > On 10 January 2015 at 08:47, Bin Meng <bmeng.cn@gmail.com> wrote: >> Hi Simon, >> >> On Fri, Jan 9, 2015 at 11:30 AM, Simon Glass <sjg@chromium.org> wrote: >>> The existing IP checksum function is only accessible to the 'coreboot' cpu. >>> Move it into the common area and rename it slightly to remove the >>> abbreviations. >>> >>> Signed-off-by: Simon Glass <sjg@chromium.org> >>> --- >>> >>> Changes in v2: >>> - Refactor IP checksum patches >>> >>> arch/x86/cpu/Makefile | 1 + >>> arch/x86/cpu/coreboot/Makefile | 1 - >>> arch/x86/cpu/coreboot/ipchecksum.c | 55 ------------------------- >>> arch/x86/cpu/coreboot/tables.c | 8 ++-- >>> arch/x86/cpu/ip_checksum.c | 34 +++++++++++++++ >> >> What about the namings we discussed in the v1 patch thread? ip_xxx >> indicates ip protocol, but acutally they are not. > > Is it not? This is not actually CMOS-specific - e.g. it is used by > Coreboot to send data through to U-Boot in the tables it provides. > > I thought it was an IP checksum... > OK, so you mean coreboot is using the same IP checksum algorithm to generate checksums for coreboot tables to be passed to U-Boot. If that is the case, I think we should put the these files to u-boot/net or u-boot/lib. Are there existing codes that we can reuse? Regards, Bin
Hi Bin, On 10 January 2015 at 19:44, Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Simon, > > On Sun, Jan 11, 2015 at 1:20 AM, Simon Glass <sjg@chromium.org> wrote: > > Hi Bin, > > > > On 10 January 2015 at 08:47, Bin Meng <bmeng.cn@gmail.com> wrote: > >> Hi Simon, > >> > >> On Fri, Jan 9, 2015 at 11:30 AM, Simon Glass <sjg@chromium.org> wrote: > >>> The existing IP checksum function is only accessible to the 'coreboot' cpu. > >>> Move it into the common area and rename it slightly to remove the > >>> abbreviations. > >>> > >>> Signed-off-by: Simon Glass <sjg@chromium.org> > >>> --- > >>> > >>> Changes in v2: > >>> - Refactor IP checksum patches > >>> > >>> arch/x86/cpu/Makefile | 1 + > >>> arch/x86/cpu/coreboot/Makefile | 1 - > >>> arch/x86/cpu/coreboot/ipchecksum.c | 55 ------------------------- > >>> arch/x86/cpu/coreboot/tables.c | 8 ++-- > >>> arch/x86/cpu/ip_checksum.c | 34 +++++++++++++++ > >> > >> What about the namings we discussed in the v1 patch thread? ip_xxx > >> indicates ip protocol, but acutally they are not. > > > > Is it not? This is not actually CMOS-specific - e.g. it is used by > > Coreboot to send data through to U-Boot in the tables it provides. > > > > I thought it was an IP checksum... > > > > OK, so you mean coreboot is using the same IP checksum algorithm to > generate checksums for coreboot tables to be passed to U-Boot. If that > is the case, I think we should put the these files to u-boot/net or > u-boot/lib. Are there existing codes that we can reuse? NetCkSum() is similar but does not need to worry about an odd length. So I doubt I can use that. Regards, Simon
Hi Simon, On Sun, Jan 11, 2015 at 11:33 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Bin, > > On 10 January 2015 at 19:44, Bin Meng <bmeng.cn@gmail.com> wrote: >> >> Hi Simon, >> >> On Sun, Jan 11, 2015 at 1:20 AM, Simon Glass <sjg@chromium.org> wrote: >> > Hi Bin, >> > >> > On 10 January 2015 at 08:47, Bin Meng <bmeng.cn@gmail.com> wrote: >> >> Hi Simon, >> >> >> >> On Fri, Jan 9, 2015 at 11:30 AM, Simon Glass <sjg@chromium.org> wrote: >> >>> The existing IP checksum function is only accessible to the 'coreboot' cpu. >> >>> Move it into the common area and rename it slightly to remove the >> >>> abbreviations. >> >>> >> >>> Signed-off-by: Simon Glass <sjg@chromium.org> >> >>> --- >> >>> >> >>> Changes in v2: >> >>> - Refactor IP checksum patches >> >>> >> >>> arch/x86/cpu/Makefile | 1 + >> >>> arch/x86/cpu/coreboot/Makefile | 1 - >> >>> arch/x86/cpu/coreboot/ipchecksum.c | 55 ------------------------- >> >>> arch/x86/cpu/coreboot/tables.c | 8 ++-- >> >>> arch/x86/cpu/ip_checksum.c | 34 +++++++++++++++ >> >> >> >> What about the namings we discussed in the v1 patch thread? ip_xxx >> >> indicates ip protocol, but acutally they are not. >> > >> > Is it not? This is not actually CMOS-specific - e.g. it is used by >> > Coreboot to send data through to U-Boot in the tables it provides. >> > >> > I thought it was an IP checksum... >> > >> >> OK, so you mean coreboot is using the same IP checksum algorithm to >> generate checksums for coreboot tables to be passed to U-Boot. If that >> is the case, I think we should put the these files to u-boot/net or >> u-boot/lib. Are there existing codes that we can reuse? > > NetCkSum() is similar but does not need to worry about an odd length. > So I doubt I can use that. > But would you consider moving it to some another place more suitable than arch/x86/cpu/? I feel that it is IP related stuff and it may make people confused when seeing it in arch/x86 directory. If we move it to u-boot/net, maybe you can replace NetCkSum() with the one we have here since it can deal with odd length. Regards, Bin
Hi Bin, On 10 January 2015 at 20:04, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Sun, Jan 11, 2015 at 11:33 AM, Simon Glass <sjg@chromium.org> wrote: >> Hi Bin, >> >> On 10 January 2015 at 19:44, Bin Meng <bmeng.cn@gmail.com> wrote: >>> >>> Hi Simon, >>> >>> On Sun, Jan 11, 2015 at 1:20 AM, Simon Glass <sjg@chromium.org> wrote: >>> > Hi Bin, >>> > >>> > On 10 January 2015 at 08:47, Bin Meng <bmeng.cn@gmail.com> wrote: >>> >> Hi Simon, >>> >> >>> >> On Fri, Jan 9, 2015 at 11:30 AM, Simon Glass <sjg@chromium.org> wrote: >>> >>> The existing IP checksum function is only accessible to the 'coreboot' cpu. >>> >>> Move it into the common area and rename it slightly to remove the >>> >>> abbreviations. >>> >>> >>> >>> Signed-off-by: Simon Glass <sjg@chromium.org> >>> >>> --- >>> >>> >>> >>> Changes in v2: >>> >>> - Refactor IP checksum patches >>> >>> >>> >>> arch/x86/cpu/Makefile | 1 + >>> >>> arch/x86/cpu/coreboot/Makefile | 1 - >>> >>> arch/x86/cpu/coreboot/ipchecksum.c | 55 ------------------------- >>> >>> arch/x86/cpu/coreboot/tables.c | 8 ++-- >>> >>> arch/x86/cpu/ip_checksum.c | 34 +++++++++++++++ >>> >> >>> >> What about the namings we discussed in the v1 patch thread? ip_xxx >>> >> indicates ip protocol, but acutally they are not. >>> > >>> > Is it not? This is not actually CMOS-specific - e.g. it is used by >>> > Coreboot to send data through to U-Boot in the tables it provides. >>> > >>> > I thought it was an IP checksum... >>> > >>> >>> OK, so you mean coreboot is using the same IP checksum algorithm to >>> generate checksums for coreboot tables to be passed to U-Boot. If that >>> is the case, I think we should put the these files to u-boot/net or >>> u-boot/lib. Are there existing codes that we can reuse? >> >> NetCkSum() is similar but does not need to worry about an odd length. >> So I doubt I can use that. >> > > But would you consider moving it to some another place more suitable > than arch/x86/cpu/? I feel that it is IP related stuff and it may make > people confused when seeing it in arch/x86 directory. If we move it to > u-boot/net, maybe you can replace NetCkSum() with the one we have here > since it can deal with odd length. I suspect it might be confusing in any case. But I think we can move this file to net/ - at least I don't see any big problems. I wonder whether people might complain about changing the existing NetChkSum() function, but we will see. Regards, Simon
diff --git a/arch/x86/cpu/Makefile b/arch/x86/cpu/Makefile index 62e43c0..eee2289 100644 --- a/arch/x86/cpu/Makefile +++ b/arch/x86/cpu/Makefile @@ -16,6 +16,7 @@ obj-$(CONFIG_SYS_COREBOOT) += coreboot/ obj-$(CONFIG_NORTHBRIDGE_INTEL_SANDYBRIDGE) += ivybridge/ obj-$(CONFIG_NORTHBRIDGE_INTEL_IVYBRIDGE) += ivybridge/ obj-$(CONFIG_INTEL_QUEENSBAY) += queensbay/ +obj-y += ip_checksum.o obj-y += lapic.o obj-y += mtrr.o obj-$(CONFIG_PCI) += pci.o diff --git a/arch/x86/cpu/coreboot/Makefile b/arch/x86/cpu/coreboot/Makefile index 35e6cdd..b6e870a 100644 --- a/arch/x86/cpu/coreboot/Makefile +++ b/arch/x86/cpu/coreboot/Makefile @@ -16,7 +16,6 @@ obj-y += car.o obj-y += coreboot.o obj-y += tables.o -obj-y += ipchecksum.o obj-y += sdram.o obj-y += timestamp.o obj-$(CONFIG_PCI) += pci.o diff --git a/arch/x86/cpu/coreboot/ipchecksum.c b/arch/x86/cpu/coreboot/ipchecksum.c deleted file mode 100644 index 3340872..0000000 --- a/arch/x86/cpu/coreboot/ipchecksum.c +++ /dev/null @@ -1,55 +0,0 @@ -/* - * This file is part of the libpayload project. - * - * It has originally been taken from the FreeBSD project. - * - * Copyright (c) 2001 Charles Mott <cm@linktel.net> - * Copyright (c) 2008 coresystems GmbH - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * 1. Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in the - * documentation and/or other materials provided with the distribution. - * - * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND - * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE - * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL - * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS - * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) - * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT - * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY - * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF - * SUCH DAMAGE. - */ - -#include <linux/types.h> -#include <linux/compiler.h> -#include <asm/arch/ipchecksum.h> - -unsigned short ipchksum(const void *vptr, unsigned long nbytes) -{ - int sum, oddbyte; - const unsigned short *ptr = vptr; - - sum = 0; - while (nbytes > 1) { - sum += *ptr++; - nbytes -= 2; - } - if (nbytes == 1) { - oddbyte = 0; - ((u8 *)&oddbyte)[0] = *(u8 *) ptr; - ((u8 *)&oddbyte)[1] = 0; - sum += oddbyte; - } - sum = (sum >> 16) + (sum & 0xffff); - sum += (sum >> 16); - return ~sum; -} diff --git a/arch/x86/cpu/coreboot/tables.c b/arch/x86/cpu/coreboot/tables.c index 92b7528..662af95 100644 --- a/arch/x86/cpu/coreboot/tables.c +++ b/arch/x86/cpu/coreboot/tables.c @@ -8,7 +8,7 @@ */ #include <common.h> -#include <asm/arch/ipchecksum.h> +#include <asm/ip_checksum.h> #include <asm/arch/sysinfo.h> #include <asm/arch/tables.h> @@ -131,11 +131,11 @@ static int cb_parse_header(void *addr, int len, struct sysinfo_t *info) return 0; /* Make sure the checksums match. */ - if (ipchksum((u16 *) header, sizeof(*header)) != 0) + if (compute_ip_checksum((u16 *)header, sizeof(*header)) != 0) return -1; - if (ipchksum((u16 *) (ptr + sizeof(*header)), - header->table_bytes) != header->table_checksum) + if (compute_ip_checksum((u16 *)(ptr + sizeof(*header)), + header->table_bytes) != header->table_checksum) return -1; /* Now, walk the tables. */ diff --git a/arch/x86/cpu/ip_checksum.c b/arch/x86/cpu/ip_checksum.c new file mode 100644 index 0000000..1b529da --- /dev/null +++ b/arch/x86/cpu/ip_checksum.c @@ -0,0 +1,34 @@ +/* + * This file was originally taken from the FreeBSD project. + * + * Copyright (c) 2001 Charles Mott <cm@linktel.net> + * Copyright (c) 2008 coresystems GmbH + * All rights reserved. + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include <linux/types.h> +#include <linux/compiler.h> +#include <asm/ip_checksum.h> + +unsigned short compute_ip_checksum(const void *vptr, unsigned long nbytes) +{ + int sum, oddbyte; + const unsigned short *ptr = vptr; + + sum = 0; + while (nbytes > 1) { + sum += *ptr++; + nbytes -= 2; + } + if (nbytes == 1) { + oddbyte = 0; + ((u8 *)&oddbyte)[0] = *(u8 *)ptr; + ((u8 *)&oddbyte)[1] = 0; + sum += oddbyte; + } + sum = (sum >> 16) + (sum & 0xffff); + sum += (sum >> 16); + return ~sum; +} diff --git a/arch/x86/include/asm/arch-coreboot/ipchecksum.h b/arch/x86/include/asm/arch-coreboot/ipchecksum.h deleted file mode 100644 index 1d73b4d..0000000 --- a/arch/x86/include/asm/arch-coreboot/ipchecksum.h +++ /dev/null @@ -1,37 +0,0 @@ -/* - * This file is part of the libpayload project. - * - * It has originally been taken from the FreeBSD project. - * - * Copyright (c) 2001 Charles Mott <cm@linktel.net> - * Copyright (c) 2008 coresystems GmbH - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * 1. Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in the - * documentation and/or other materials provided with the distribution. - * - * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND - * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE - * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL - * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS - * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) - * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT - * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY - * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF - * SUCH DAMAGE. - */ - -#ifndef _COREBOOT_IPCHECKSUM_H -#define _COREBOOT_IPCHECKSUM_H - -unsigned short ipchksum(const void *vptr, unsigned long nbytes); - -#endif diff --git a/arch/x86/include/asm/ip_checksum.h b/arch/x86/include/asm/ip_checksum.h new file mode 100644 index 0000000..8d9626d --- /dev/null +++ b/arch/x86/include/asm/ip_checksum.h @@ -0,0 +1,16 @@ +/* + * From Coreboot ip_checksum.h + * + * Copyright (c) 2014 Google, Inc + * + * SPDX-License-Identifier: GPL-2.0 + */ + +#ifndef _IP_CHECKSUM_H +#define _IP_CHECKSUM_H + +unsigned short compute_ip_checksum(const void *addr, unsigned long length); +unsigned long add_ip_checksums(unsigned long offset, unsigned long sum, + unsigned long new); + +#endif /* IP_CHECKSUM_H */
The existing IP checksum function is only accessible to the 'coreboot' cpu. Move it into the common area and rename it slightly to remove the abbreviations. Signed-off-by: Simon Glass <sjg@chromium.org> --- Changes in v2: - Refactor IP checksum patches arch/x86/cpu/Makefile | 1 + arch/x86/cpu/coreboot/Makefile | 1 - arch/x86/cpu/coreboot/ipchecksum.c | 55 ------------------------- arch/x86/cpu/coreboot/tables.c | 8 ++-- arch/x86/cpu/ip_checksum.c | 34 +++++++++++++++ arch/x86/include/asm/arch-coreboot/ipchecksum.h | 37 ----------------- arch/x86/include/asm/ip_checksum.h | 16 +++++++ 7 files changed, 55 insertions(+), 97 deletions(-) delete mode 100644 arch/x86/cpu/coreboot/ipchecksum.c create mode 100644 arch/x86/cpu/ip_checksum.c delete mode 100644 arch/x86/include/asm/arch-coreboot/ipchecksum.h create mode 100644 arch/x86/include/asm/ip_checksum.h