diff mbox series

[v4,3/5] tools/libs/foreignmemory: Fix PAGE_SIZE redefinition error

Message ID 83beb95e3633b1aca7801fd8592406e2057f9bdc.1623155575.git.costin.lupu@cs.pub.ro (mailing list archive)
State New
Headers show
Series Fix redefinition errors for toolstack libs | expand

Commit Message

Costin Lupu June 8, 2021, 12:35 p.m. UTC
If PAGE_SIZE is already defined in the system (e.g. in /usr/include/limits.h
header) then gcc will trigger a redefinition error because of -Werror. This
patch replaces usage of PAGE_* macros with XC_PAGE_* macros in order to avoid
confusion between control domain page granularity (PAGE_* definitions) and
guest domain page granularity.

The exception is in osdep_xenforeignmemory_map() where we need the system page
size to check whether the PFN array should be allocated with mmap() or with
dynamic allocation.

Signed-off-by: Costin Lupu <costin.lupu@cs.pub.ro>
---
 tools/libs/foreignmemory/core.c    |  2 +-
 tools/libs/foreignmemory/freebsd.c | 10 +++++-----
 tools/libs/foreignmemory/linux.c   | 23 ++++++++++++-----------
 tools/libs/foreignmemory/minios.c  |  2 +-
 tools/libs/foreignmemory/netbsd.c  | 10 +++++-----
 tools/libs/foreignmemory/private.h |  9 +--------
 6 files changed, 25 insertions(+), 31 deletions(-)

Comments

Julien Grall July 8, 2021, 5:30 p.m. UTC | #1
Hi Costin,

On 08/06/2021 13:35, Costin Lupu wrote:
> If PAGE_SIZE is already defined in the system (e.g. in /usr/include/limits.h
> header) then gcc will trigger a redefinition error because of -Werror. This
> patch replaces usage of PAGE_* macros with XC_PAGE_* macros in order to avoid
> confusion between control domain page granularity (PAGE_* definitions) and
> guest domain page granularity.
> 
> The exception is in osdep_xenforeignmemory_map() where we need the system page
> size to check whether the PFN array should be allocated with mmap() or with
> dynamic allocation.
> 
> Signed-off-by: Costin Lupu <costin.lupu@cs.pub.ro>

Reviewed-by: Julien Grall <jgrall@amazon.com>

Cheers,
Jan Beulich July 13, 2021, 6:47 a.m. UTC | #2
On 08.06.2021 14:35, Costin Lupu wrote:
> --- a/tools/libs/foreignmemory/private.h
> +++ b/tools/libs/foreignmemory/private.h
> @@ -1,6 +1,7 @@
>  #ifndef XENFOREIGNMEMORY_PRIVATE_H
>  #define XENFOREIGNMEMORY_PRIVATE_H
>  
> +#include <xenctrl.h>
>  #include <xentoollog.h>
>  
>  #include <xenforeignmemory.h>

At the risk of repeating what may have been discussed on irc already yesterday
(which I would not have seen), this is the cause for the present smoke test
failure:

In file included from /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:39,
                 from /home/osstest/build.163627.build-amd64/xen/tools/include/xenctrl.h:36,
                 from private.h:4,
                 from minios.c:29:
/home/osstest/build.163627.build-amd64/xen/xen/include/public/memory.h:407:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(const_uint8) buffer;
     ^~~~~~~~~~~~~~~~~~~
In file included from /home/osstest/build.163627.build-amd64/xen/tools/include/xenctrl.h:36,
                 from private.h:4,
                 from minios.c:29:
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:101:34: error: field 'arch' has incomplete type
     struct xen_arch_domainconfig arch;
                                  ^~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:152:34: error: field 'arch_config' has incomplete type
     struct xen_arch_domainconfig arch_config;
                                  ^~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:182:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(xen_pfn_t) array;
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:263:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(uint8) dirty_bitmap;
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:280:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(vcpu_guest_context_t) ctxt; /* IN/OUT */
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:301:26: error: field 'nodemap' has incomplete type
     struct xenctl_bitmap nodemap;/* IN */
                          ^~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:337:26: error: field 'cpumap_hard' has incomplete type
     struct xenctl_bitmap cpumap_hard;
                          ^~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:338:26: error: field 'cpumap_soft' has incomplete type
     struct xenctl_bitmap cpumap_soft;
                          ^~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:418:13: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
             XEN_GUEST_HANDLE_64(xen_domctl_schedparam_vcpu_t) vcpus;
             ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:473:5: error: unknown type name 'int64_aligned_t'
     int64_aligned_t time_offset_seconds; /* applied to domain wallclock time */
     ^~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:480:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(uint8) buffer; /* IN/OUT: data, or call
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:533:13: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
             XEN_GUEST_HANDLE_64(char) path; /* path to the device tree node */
             ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:544:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(uint32)  sdev_array;   /* OUT */
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:685:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(xen_cpuid_leaf_t) cpuid_policy; /* IN/OUT */
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:735:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(uint8) buffer;  /* OUT: buffer to write record into */
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:909:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(uint64) buffer;
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:963:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(xen_domctl_vcpu_msr_t) msrs; /* IN/OUT */
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:984:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(uint) vdistance;
     ^~~~~~~~~~~~~~~~~~~
In file included from /home/osstest/build.163627.build-amd64/xen/tools/include/xenctrl.h:38,
                 from private.h:4,
                 from minios.c:29:
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:56:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(char) buffer;
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:73:26: error: field 'cpu_mask' has incomplete type
     struct xenctl_bitmap cpu_mask;
                          ^~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:155:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(xen_sysctl_perfc_desc_t) desc;
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:165:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(xen_domctl_getdomaininfo_t) buffer;
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:174:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(const_char) keys;
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:188:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(xen_sysctl_cpuinfo_t) info;
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:217:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(uint64) trans_pt;   /* Px transition table */
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:225:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(uint64) triggers;    /* Cx trigger counts */
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:317:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(uint32) affected_cpus;
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:474:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(xen_sysctl_lockprof_data_t) data;
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:504:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(xen_sysctl_cputopo_t) cputopo;
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:537:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(xen_sysctl_meminfo_t) meminfo;
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:563:26: error: field 'cpumap' has incomplete type
     struct xenctl_bitmap cpumap; /*     OUT: IF */
                          ^~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:665:13: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
             XEN_GUEST_HANDLE_64(xen_sysctl_arinc653_schedule_t) schedule;
             ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:707:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(char) buffer; /* OUT */
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:738:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(physdev_pci_device_t) devs;
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:814:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(uint32) features; /* OUT: */
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:887:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(char) name;         /* IN: pointer to name. */
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:912:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(uint8) payload;     /* IN, the ELF file. */
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:975:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(xen_livepatch_status_t) status;  /* OUT. Must have enough
     ^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:1059:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
     XEN_GUEST_HANDLE_64(xen_cpuid_leaf_t) cpuid_policy; /* OUT */
     ^~~~~~~~~~~~~~~~~~~
In file included from /home/osstest/build.163627.build-amd64/xen/tools/include/xenctrl.h:55,
                 from private.h:4,
                 from minios.c:29:
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/arch-x86/xen-mca.h:431:5: error: unknown type name 'xenctl_bitmap_t'
     xenctl_bitmap_t cpumap;
     ^~~~~~~~~~~~~~~
In file included from private.h:4,
                 from minios.c:29:
/home/osstest/build.163627.build-amd64/xen/tools/include/xenctrl.h:468:34: error: field 'arch_config' has incomplete type
     struct xen_arch_domainconfig arch_config;
                                  ^~~~~~~~~~~

Clearly xenctrl.h cannot be included freely right now; it expects other
header to have been included first. Question is whether that's what needs
fixing, or whether the new #include wants prefixing by whatever prereq
headers that are needed. Or whether, considering that libxenforeignmemory.so
doesn't depend on libxc.so, including xenctrl.h is inappropriate here in the
first place, meaning that the tool stack's PAGE_SIZE abstraction may want to
move to a separate header which is not tied to any particular library.

Jan
Costin Lupu July 13, 2021, 11:51 a.m. UTC | #3
Hi Jan,

First of all sorry for breaking the stubdom build. Please see inline.

On 7/13/21 9:47 AM, Jan Beulich wrote:
> On 08.06.2021 14:35, Costin Lupu wrote:
>> --- a/tools/libs/foreignmemory/private.h
>> +++ b/tools/libs/foreignmemory/private.h
>> @@ -1,6 +1,7 @@
>>  #ifndef XENFOREIGNMEMORY_PRIVATE_H
>>  #define XENFOREIGNMEMORY_PRIVATE_H
>>  
>> +#include <xenctrl.h>
>>  #include <xentoollog.h>
>>  
>>  #include <xenforeignmemory.h>
> 
> At the risk of repeating what may have been discussed on irc already yesterday
> (which I would not have seen), this is the cause for the present smoke test
> failure:
> 
> In file included from /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:39,
>                  from /home/osstest/build.163627.build-amd64/xen/tools/include/xenctrl.h:36,
>                  from private.h:4,
>                  from minios.c:29:
> /home/osstest/build.163627.build-amd64/xen/xen/include/public/memory.h:407:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(const_uint8) buffer;
>      ^~~~~~~~~~~~~~~~~~~
> In file included from /home/osstest/build.163627.build-amd64/xen/tools/include/xenctrl.h:36,
>                  from private.h:4,
>                  from minios.c:29:
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:101:34: error: field 'arch' has incomplete type
>      struct xen_arch_domainconfig arch;
>                                   ^~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:152:34: error: field 'arch_config' has incomplete type
>      struct xen_arch_domainconfig arch_config;
>                                   ^~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:182:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(xen_pfn_t) array;
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:263:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(uint8) dirty_bitmap;
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:280:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(vcpu_guest_context_t) ctxt; /* IN/OUT */
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:301:26: error: field 'nodemap' has incomplete type
>      struct xenctl_bitmap nodemap;/* IN */
>                           ^~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:337:26: error: field 'cpumap_hard' has incomplete type
>      struct xenctl_bitmap cpumap_hard;
>                           ^~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:338:26: error: field 'cpumap_soft' has incomplete type
>      struct xenctl_bitmap cpumap_soft;
>                           ^~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:418:13: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>              XEN_GUEST_HANDLE_64(xen_domctl_schedparam_vcpu_t) vcpus;
>              ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:473:5: error: unknown type name 'int64_aligned_t'
>      int64_aligned_t time_offset_seconds; /* applied to domain wallclock time */
>      ^~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:480:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(uint8) buffer; /* IN/OUT: data, or call
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:533:13: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>              XEN_GUEST_HANDLE_64(char) path; /* path to the device tree node */
>              ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:544:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(uint32)  sdev_array;   /* OUT */
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:685:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(xen_cpuid_leaf_t) cpuid_policy; /* IN/OUT */
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:735:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(uint8) buffer;  /* OUT: buffer to write record into */
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:909:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(uint64) buffer;
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:963:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(xen_domctl_vcpu_msr_t) msrs; /* IN/OUT */
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:984:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(uint) vdistance;
>      ^~~~~~~~~~~~~~~~~~~
> In file included from /home/osstest/build.163627.build-amd64/xen/tools/include/xenctrl.h:38,
>                  from private.h:4,
>                  from minios.c:29:
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:56:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(char) buffer;
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:73:26: error: field 'cpu_mask' has incomplete type
>      struct xenctl_bitmap cpu_mask;
>                           ^~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:155:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(xen_sysctl_perfc_desc_t) desc;
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:165:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(xen_domctl_getdomaininfo_t) buffer;
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:174:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(const_char) keys;
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:188:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(xen_sysctl_cpuinfo_t) info;
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:217:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(uint64) trans_pt;   /* Px transition table */
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:225:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(uint64) triggers;    /* Cx trigger counts */
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:317:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(uint32) affected_cpus;
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:474:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(xen_sysctl_lockprof_data_t) data;
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:504:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(xen_sysctl_cputopo_t) cputopo;
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:537:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(xen_sysctl_meminfo_t) meminfo;
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:563:26: error: field 'cpumap' has incomplete type
>      struct xenctl_bitmap cpumap; /*     OUT: IF */
>                           ^~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:665:13: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>              XEN_GUEST_HANDLE_64(xen_sysctl_arinc653_schedule_t) schedule;
>              ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:707:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(char) buffer; /* OUT */
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:738:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(physdev_pci_device_t) devs;
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:814:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(uint32) features; /* OUT: */
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:887:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(char) name;         /* IN: pointer to name. */
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:912:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(uint8) payload;     /* IN, the ELF file. */
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:975:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(xen_livepatch_status_t) status;  /* OUT. Must have enough
>      ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:1059:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
>      XEN_GUEST_HANDLE_64(xen_cpuid_leaf_t) cpuid_policy; /* OUT */
>      ^~~~~~~~~~~~~~~~~~~
> In file included from /home/osstest/build.163627.build-amd64/xen/tools/include/xenctrl.h:55,
>                  from private.h:4,
>                  from minios.c:29:
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/arch-x86/xen-mca.h:431:5: error: unknown type name 'xenctl_bitmap_t'
>      xenctl_bitmap_t cpumap;
>      ^~~~~~~~~~~~~~~
> In file included from private.h:4,
>                  from minios.c:29:
> /home/osstest/build.163627.build-amd64/xen/tools/include/xenctrl.h:468:34: error: field 'arch_config' has incomplete type
>      struct xen_arch_domainconfig arch_config;
>                                   ^~~~~~~~~~~
> 
> Clearly xenctrl.h cannot be included freely right now; it expects other
> header to have been included first. Question is whether that's what needs
> fixing, or whether the new #include wants prefixing by whatever prereq
> headers that are needed. Or whether, considering that libxenforeignmemory.so
> doesn't depend on libxc.so, including xenctrl.h is inappropriate here in the
> first place, meaning that the tool stack's PAGE_SIZE abstraction may want to
> move to a separate header which is not tied to any particular library.
> 

XEN_GUEST_HANDLE_64 is defined in xen.h. The xenctrl.h header does
include xen.h before including memory.h, where XEN_GUEST_HANDLE_64 is
used. However, xen.h is also included before that inclusion by
mini-os/os.h (included by minios.c) without defining __XEN_TOOLS__ and
thus disabling the definition of XEN_GUEST_HANDLE_64 from xen.h.

Although moving the PAGE_SIZE abstraction definitions in a header of its
own would fix this build issue, we would still have the problem
described above, i.e. we include xen.h without defining __XEN_TOOLS__,
but we would need toolstack definitions. This is a bit unclear for me,
shouldn't __XEN_TOOLS__ be defined for a stubdom?

Having said that, I can create a new header for the toolstack PAGE_SIZE
abstraction, but I would need some name suggestions. Would
xenctrl_page.h be ok? Would we keep the XC_PAGE_* names if we consider
that they will be toolstack specific (and not tied to xenctrl lib)?.


Cheers,
Costin
diff mbox series

Patch

diff --git a/tools/libs/foreignmemory/core.c b/tools/libs/foreignmemory/core.c
index 28ec311af1..7edc6f0dbf 100644
--- a/tools/libs/foreignmemory/core.c
+++ b/tools/libs/foreignmemory/core.c
@@ -202,7 +202,7 @@  int xenforeignmemory_resource_size(
     if ( rc )
         return rc;
 
-    *size = fres.nr_frames << PAGE_SHIFT;
+    *size = fres.nr_frames << XC_PAGE_SHIFT;
     return 0;
 }
 
diff --git a/tools/libs/foreignmemory/freebsd.c b/tools/libs/foreignmemory/freebsd.c
index d94ea07862..2cf0fa1c38 100644
--- a/tools/libs/foreignmemory/freebsd.c
+++ b/tools/libs/foreignmemory/freebsd.c
@@ -63,7 +63,7 @@  void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
     privcmd_mmapbatch_t ioctlx;
     int rc;
 
-    addr = mmap(addr, num << PAGE_SHIFT, prot, flags | MAP_SHARED, fd, 0);
+    addr = mmap(addr, num << XC_PAGE_SHIFT, prot, flags | MAP_SHARED, fd, 0);
     if ( addr == MAP_FAILED )
         return NULL;
 
@@ -78,7 +78,7 @@  void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
     {
         int saved_errno = errno;
 
-        (void)munmap(addr, num << PAGE_SHIFT);
+        (void)munmap(addr, num << XC_PAGE_SHIFT);
         errno = saved_errno;
         return NULL;
     }
@@ -89,7 +89,7 @@  void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
 int osdep_xenforeignmemory_unmap(xenforeignmemory_handle *fmem,
                                  void *addr, size_t num)
 {
-    return munmap(addr, num << PAGE_SHIFT);
+    return munmap(addr, num << XC_PAGE_SHIFT);
 }
 
 int osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
@@ -101,7 +101,7 @@  int osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
 int osdep_xenforeignmemory_unmap_resource(xenforeignmemory_handle *fmem,
                                         xenforeignmemory_resource_handle *fres)
 {
-    return fres ? munmap(fres->addr, fres->nr_frames << PAGE_SHIFT) : 0;
+    return fres ? munmap(fres->addr, fres->nr_frames << XC_PAGE_SHIFT) : 0;
 }
 
 int osdep_xenforeignmemory_map_resource(xenforeignmemory_handle *fmem,
@@ -120,7 +120,7 @@  int osdep_xenforeignmemory_map_resource(xenforeignmemory_handle *fmem,
         /* Request for resource size.  Skip mmap(). */
         goto skip_mmap;
 
-    fres->addr = mmap(fres->addr, fres->nr_frames << PAGE_SHIFT,
+    fres->addr = mmap(fres->addr, fres->nr_frames << XC_PAGE_SHIFT,
                       fres->prot, fres->flags | MAP_SHARED, fmem->fd, 0);
     if ( fres->addr == MAP_FAILED )
         return -1;
diff --git a/tools/libs/foreignmemory/linux.c b/tools/libs/foreignmemory/linux.c
index c1f35e2db7..9062117407 100644
--- a/tools/libs/foreignmemory/linux.c
+++ b/tools/libs/foreignmemory/linux.c
@@ -134,7 +134,7 @@  static int retry_paged(int fd, uint32_t dom, void *addr,
         /* At least one gfn is still in paging state */
         ioctlx.num = 1;
         ioctlx.dom = dom;
-        ioctlx.addr = (unsigned long)addr + (i<<PAGE_SHIFT);
+        ioctlx.addr = (unsigned long)addr + (i<<XC_PAGE_SHIFT);
         ioctlx.arr = arr + i;
         ioctlx.err = err + i;
 
@@ -168,7 +168,7 @@  void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
     size_t i;
     int rc;
 
-    addr = mmap(addr, num << PAGE_SHIFT, prot, flags | MAP_SHARED,
+    addr = mmap(addr, num << XC_PAGE_SHIFT, prot, flags | MAP_SHARED,
                 fd, 0);
     if ( addr == MAP_FAILED )
         return NULL;
@@ -198,9 +198,10 @@  void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
          */
         privcmd_mmapbatch_t ioctlx;
         xen_pfn_t *pfn;
-        unsigned int pfn_arr_size = ROUNDUP((num * sizeof(*pfn)), PAGE_SHIFT);
+        unsigned int pfn_arr_size = ROUNDUP((num * sizeof(*pfn)), XC_PAGE_SHIFT);
+        int os_page_size = sysconf(_SC_PAGESIZE);
 
-        if ( pfn_arr_size <= PAGE_SIZE )
+        if ( pfn_arr_size <= os_page_size )
             pfn = alloca(num * sizeof(*pfn));
         else
         {
@@ -209,7 +210,7 @@  void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
             if ( pfn == MAP_FAILED )
             {
                 PERROR("mmap of pfn array failed");
-                (void)munmap(addr, num << PAGE_SHIFT);
+                (void)munmap(addr, num << XC_PAGE_SHIFT);
                 return NULL;
             }
         }
@@ -242,7 +243,7 @@  void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
                     continue;
                 }
                 rc = map_foreign_batch_single(fd, dom, pfn + i,
-                        (unsigned long)addr + (i<<PAGE_SHIFT));
+                        (unsigned long)addr + (i<<XC_PAGE_SHIFT));
                 if ( rc < 0 )
                 {
                     rc = -errno;
@@ -254,7 +255,7 @@  void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
             break;
         }
 
-        if ( pfn_arr_size > PAGE_SIZE )
+        if ( pfn_arr_size > os_page_size )
             munmap(pfn, pfn_arr_size);
 
         if ( rc == -ENOENT && i == num )
@@ -270,7 +271,7 @@  void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
     {
         int saved_errno = errno;
 
-        (void)munmap(addr, num << PAGE_SHIFT);
+        (void)munmap(addr, num << XC_PAGE_SHIFT);
         errno = saved_errno;
         return NULL;
     }
@@ -281,7 +282,7 @@  void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
 int osdep_xenforeignmemory_unmap(xenforeignmemory_handle *fmem,
                                  void *addr, size_t num)
 {
-    return munmap(addr, num << PAGE_SHIFT);
+    return munmap(addr, num << XC_PAGE_SHIFT);
 }
 
 int osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
@@ -293,7 +294,7 @@  int osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
 int osdep_xenforeignmemory_unmap_resource(
     xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres)
 {
-    return fres ? munmap(fres->addr, fres->nr_frames << PAGE_SHIFT) : 0;
+    return fres ? munmap(fres->addr, fres->nr_frames << XC_PAGE_SHIFT) : 0;
 }
 
 int osdep_xenforeignmemory_map_resource(
@@ -312,7 +313,7 @@  int osdep_xenforeignmemory_map_resource(
         /* Request for resource size.  Skip mmap(). */
         goto skip_mmap;
 
-    fres->addr = mmap(fres->addr, fres->nr_frames << PAGE_SHIFT,
+    fres->addr = mmap(fres->addr, fres->nr_frames << XC_PAGE_SHIFT,
                       fres->prot, fres->flags | MAP_SHARED, fmem->fd, 0);
     if ( fres->addr == MAP_FAILED )
         return -1;
diff --git a/tools/libs/foreignmemory/minios.c b/tools/libs/foreignmemory/minios.c
index 43341ca301..c5453736d5 100644
--- a/tools/libs/foreignmemory/minios.c
+++ b/tools/libs/foreignmemory/minios.c
@@ -55,7 +55,7 @@  void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
 int osdep_xenforeignmemory_unmap(xenforeignmemory_handle *fmem,
                                  void *addr, size_t num)
 {
-    return munmap(addr, num << PAGE_SHIFT);
+    return munmap(addr, num << XC_PAGE_SHIFT);
 }
 
 /*
diff --git a/tools/libs/foreignmemory/netbsd.c b/tools/libs/foreignmemory/netbsd.c
index c0b1b8f79d..597db775d7 100644
--- a/tools/libs/foreignmemory/netbsd.c
+++ b/tools/libs/foreignmemory/netbsd.c
@@ -76,7 +76,7 @@  void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
 {
     int fd = fmem->fd;
     privcmd_mmapbatch_v2_t ioctlx;
-    addr = mmap(addr, num * PAGE_SIZE, prot,
+    addr = mmap(addr, num * XC_PAGE_SIZE, prot,
                 flags | MAP_ANON | MAP_SHARED, -1, 0);
     if ( addr == MAP_FAILED ) {
         PERROR("osdep_xenforeignmemory_map: mmap failed");
@@ -93,7 +93,7 @@  void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
     {
         int saved_errno = errno;
         PERROR("osdep_xenforeignmemory_map: ioctl failed");
-        munmap(addr, num * PAGE_SIZE);
+        munmap(addr, num * XC_PAGE_SIZE);
         errno = saved_errno;
         return NULL;
     }
@@ -104,7 +104,7 @@  void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
 int osdep_xenforeignmemory_unmap(xenforeignmemory_handle *fmem,
                                  void *addr, size_t num)
 {
-    return munmap(addr, num * PAGE_SIZE);
+    return munmap(addr, num * XC_PAGE_SIZE);
 }
 
 int osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
@@ -117,7 +117,7 @@  int osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
 int osdep_xenforeignmemory_unmap_resource(
     xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres)
 {
-    return fres ? munmap(fres->addr, fres->nr_frames << PAGE_SHIFT) : 0;
+    return fres ? munmap(fres->addr, fres->nr_frames << XC_PAGE_SHIFT) : 0;
 }
 
 int osdep_xenforeignmemory_map_resource(
@@ -136,7 +136,7 @@  int osdep_xenforeignmemory_map_resource(
         /* Request for resource size.  Skip mmap(). */
         goto skip_mmap;
 
-    fres->addr = mmap(fres->addr, fres->nr_frames << PAGE_SHIFT,
+    fres->addr = mmap(fres->addr, fres->nr_frames << XC_PAGE_SHIFT,
                       fres->prot, fres->flags | MAP_ANON | MAP_SHARED, -1, 0);
     if ( fres->addr == MAP_FAILED )
         return -1;
diff --git a/tools/libs/foreignmemory/private.h b/tools/libs/foreignmemory/private.h
index 1ee3626dd2..65fe77aa5b 100644
--- a/tools/libs/foreignmemory/private.h
+++ b/tools/libs/foreignmemory/private.h
@@ -1,6 +1,7 @@ 
 #ifndef XENFOREIGNMEMORY_PRIVATE_H
 #define XENFOREIGNMEMORY_PRIVATE_H
 
+#include <xenctrl.h>
 #include <xentoollog.h>
 
 #include <xenforeignmemory.h>
@@ -10,14 +11,6 @@ 
 #include <xen/xen.h>
 #include <xen/sys/privcmd.h>
 
-#ifndef PAGE_SHIFT /* Mini-os, Yukk */
-#define PAGE_SHIFT           12
-#endif
-#ifndef __MINIOS__ /* Yukk */
-#define PAGE_SIZE            (1UL << PAGE_SHIFT)
-#define PAGE_MASK            (~(PAGE_SIZE-1))
-#endif
-
 struct xenforeignmemory_handle {
     xentoollog_logger *logger, *logger_tofree;
     unsigned flags;