Re: [Xen-devel] [PATCH v2 13/23] acpi/hvmloader: Include file/paths adjustments

2016-08-10 Thread Boris Ostrovsky
On 08/10/2016 09:30 AM, Jan Beulich wrote:

>> +#ifndef __ACPI_X86_H__
>> +#define __ACPI_X86_H__
>> +
>> +#define IOAPIC_BASE_ADDRESS 0xfec0
> This will need re-basing - looks like your submission and my commit
> happened at about the same time.

Right, I saw that but this series is based on Anthony's tree so I
couldn't use it yet.

>
>> --- a/tools/firmware/hvmloader/hvmloader.c
>> +++ b/tools/firmware/hvmloader/hvmloader.c
>> @@ -24,8 +24,9 @@
>>  #include "config.h"
>>  #include "pci_regs.h"
>>  #include "apic_regs.h"
>> -#include "acpi/acpi2_0.h"
>> +#include "acpi2_0.h"
> Could you see to pass a suitable string via -D from the Makefile to
> prefix to headers like this one? That would clarify to the reader that
> the file being looked for is not in the current directory. The primary
> other option that I see would be to use <> instead of "", and keep
> the -I in the Makefile. Perhaps that would even be the more natural
> variant.

Yes, I'll see if that (< >) works.

-boris
>


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v2 13/23] acpi/hvmloader: Include file/paths adjustments

2016-08-10 Thread Jan Beulich
>>> On 04.08.16 at 23:06,  wrote:
> --- a/tools/firmware/hvmloader/acpi/README
> +++ b/tools/firmware/hvmloader/acpi/README
> @@ -1,11 +1,19 @@
> -ACPI Table for domain firmware
> +ACPI builder for domain firmware
>  
>  
> -INSTALL
> +BUILDING ACPI
>  -
> -Simply make is OK.
> -# make 
> +Users of ACPI builder are expected to provide an include file that defines

s/defines/makes available/ (since a declaration would fully suffice)?

> +the following:
> +* strncpy
> +* printf
> +* NULL
> +* test_bit
> +* offsetof
>  
> +When compiling build.c, the name of this include file should be given to
> +compiler as -DSTDUTILS=\"\". See tools/firmware/hvmloader/Makefile

Perhaps worthwhile using LIBACPI_STDUTILS to reduce the chance
of a name space conflict?

> +#ifndef __ACPI_X86_H__
> +#define __ACPI_X86_H__
> +
> +#define IOAPIC_BASE_ADDRESS 0xfec0

This will need re-basing - looks like your submission and my commit
happened at about the same time.

> --- a/tools/firmware/hvmloader/hvmloader.c
> +++ b/tools/firmware/hvmloader/hvmloader.c
> @@ -24,8 +24,9 @@
>  #include "config.h"
>  #include "pci_regs.h"
>  #include "apic_regs.h"
> -#include "acpi/acpi2_0.h"
> +#include "acpi2_0.h"

Could you see to pass a suitable string via -D from the Makefile to
prefix to headers like this one? That would clarify to the reader that
the file being looked for is not in the current directory. The primary
other option that I see would be to use <> instead of "", and keep
the -I in the Makefile. Perhaps that would even be the more natural
variant.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


[Xen-devel] [PATCH v2 13/23] acpi/hvmloader: Include file/paths adjustments

2016-08-04 Thread Boris Ostrovsky
In prepearation to moving acpi sources into generally available
libacpi:

1. Move certain x86-specific definitions into acpi's x86.h
2. Modify include files serach paths to point to acpi
3. Macro-ise include file for build.c that defines various
   utilities used by that file. Users of libacpi will be expected
   to define this macro when compiling build.c

Signed-off-by: Boris Ostrovsky 
---
v2:
* Note: hvmloader.c needs  due to its use of
  ACPI_PM1A_CNT_BLK_ADDRESS_V1 and ACPI_PM1C_SCI_EN

 tools/firmware/hvmloader/Makefile |  3 ++-
 tools/firmware/hvmloader/acpi/README  | 16 
 tools/firmware/hvmloader/acpi/build.c |  5 ++---
 tools/firmware/hvmloader/acpi/x86.h   | 30 ++
 tools/firmware/hvmloader/config.h |  6 --
 tools/firmware/hvmloader/hvmloader.c  |  3 ++-
 tools/firmware/hvmloader/mp_tables.c  |  1 +
 tools/firmware/hvmloader/pci.c|  1 +
 tools/firmware/hvmloader/pir.c|  1 +
 tools/firmware/hvmloader/rombios.c|  2 +-
 tools/firmware/hvmloader/seabios.c|  4 ++--
 tools/firmware/hvmloader/smp.c|  1 +
 tools/firmware/hvmloader/util.c   |  5 +++--
 13 files changed, 58 insertions(+), 20 deletions(-)
 create mode 100644 tools/firmware/hvmloader/acpi/x86.h

diff --git a/tools/firmware/hvmloader/Makefile 
b/tools/firmware/hvmloader/Makefile
index a1ab170..db64a61 100644
--- a/tools/firmware/hvmloader/Makefile
+++ b/tools/firmware/hvmloader/Makefile
@@ -76,7 +76,8 @@ smbios.o: CFLAGS += -D__SMBIOS_DATE__="\"$(SMBIOS_REL_DATE)\""
 ACPI_PATH = ./acpi
 ACPI_FILES = dsdt_anycpu.c dsdt_15cpu.c dsdt_anycpu_qemu_xen.c
 ACPI_OBJS = $(patsubst %.c,%.o,$(ACPI_FILES)) build.o static_tables.o
-$(ACPI_OBJS): CFLAGS += -I$(ACPI_PATH) -I.
+$(ACPI_OBJS): CFLAGS += -I. -DSTDUTILS=\"../util.h\"
+CFLAGS += -I$(ACPI_PATH)
 vpath build.c $(ACPI_PATH)
 vpath static_tables.c $(ACPI_PATH)
 OBJS += $(ACPI_OBJS)
diff --git a/tools/firmware/hvmloader/acpi/README 
b/tools/firmware/hvmloader/acpi/README
index 210d5ba..ef4063b 100644
--- a/tools/firmware/hvmloader/acpi/README
+++ b/tools/firmware/hvmloader/acpi/README
@@ -1,11 +1,19 @@
-ACPI Table for domain firmware
+ACPI builder for domain firmware
 
 
-INSTALL
+BUILDING ACPI
 -
-Simply make is OK.
-# make 
+Users of ACPI builder are expected to provide an include file that defines
+the following:
+* strncpy
+* printf
+* NULL
+* test_bit
+* offsetof
 
+When compiling build.c, the name of this include file should be given to
+compiler as -DSTDUTILS=\"\". See tools/firmware/hvmloader/Makefile
+for an example.
 
 Note on DSDT Table
 --
diff --git a/tools/firmware/hvmloader/acpi/build.c 
b/tools/firmware/hvmloader/acpi/build.c
index c0e0915..2432eb9 100644
--- a/tools/firmware/hvmloader/acpi/build.c
+++ b/tools/firmware/hvmloader/acpi/build.c
@@ -13,15 +13,14 @@
  * GNU Lesser General Public License for more details.
  */
 
+#include STDUTILS
 #include "acpi2_0.h"
 #include "libacpi.h"
 #include "ssdt_s3.h"
 #include "ssdt_s4.h"
 #include "ssdt_tpm.h"
 #include "ssdt_pm.h"
-#include "../config.h"
-#include "../util.h"
-#include "../vnuma.h"
+#include "x86.h"
 #include 
 #include 
 
diff --git a/tools/firmware/hvmloader/acpi/x86.h 
b/tools/firmware/hvmloader/acpi/x86.h
new file mode 100644
index 000..f305414
--- /dev/null
+++ b/tools/firmware/hvmloader/acpi/x86.h
@@ -0,0 +1,30 @@
+/**
+ * x86.h
+ *
+ * x86-specific interfaces used by ACPI builder and its callers
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ */
+
+#ifndef __ACPI_X86_H__
+#define __ACPI_X86_H__
+
+#define IOAPIC_BASE_ADDRESS 0xfec0
+#define IOAPIC_ID   0x01
+
+#define LAPIC_BASE_ADDRESS  0xfee0
+#define LAPIC_ID(vcpu_id)   ((vcpu_id) * 2)
+
+#define PCI_ISA_IRQ_MASK0x0c20U /* ISA IRQs 5,10,11 are PCI connected */
+
+#endif /* __ACPI_X86_H__ */
diff --git a/tools/firmware/hvmloader/config.h 
b/tools/firmware/hvmloader/config.h
index c9526e3..5116d96 100644
--- a/tools/firmware/hvmloader/config.h
+++ b/tools/firmware/hvmloader/config.h
@@ -42,15 +42,9 @@ extern struct bios_config ovmf_config;
 #define PAGE_SHIFT 12
 #define PAGE_SIZE  (1ul << PAGE_SHIFT)
 
-#define IOAPIC_BASE_ADDRESS 0xfec0
-#define IOAPIC_ID   0x01
 #define IOAPIC_VERSION  0x11
 
-#define LAPIC_BASE_ADDRESS  0xfee0
-#define LAPIC_ID(vcpu_id)   ((vcpu_id)