Hi Jan,
On 03/12/2020 09:27, Jan Beulich wrote:
On 02.12.2020 18:14, Julien Grall wrote:
Hi Jan,
On 02/12/2020 14:49, Jan Beulich wrote:
Doing so limits what can be done in (in particular included by) this per-
arch header. Abstract out page shift/size related #define-s, which is all
the repsecitve headers care about. Extend the replacement / removal to
s/repsecitve/respective/
some x86 headers as well; some others now need to include page.h (and
they really should have before).
Arm's VADDR_BITS gets restricted to 32-bit, as its current value is
clearly wrong for 64-bit, but the constant also isn't used anywhere
right now (i.e. the #define could also be dropped altogether).
Whoops. Thankfully this is not used.
I wasn't sure about Arm's use of vaddr_t in PAGE_OFFSET(), and hence I
kept it and provided a way to override the #define in the common header.
vaddr_t is defined to 32-bit for arm32 or 64-bit for arm64. So I think
it would be fine to use the generic PAGE_OFFSET() implementation.
Will switch.
--- /dev/null
+++ b/xen/include/asm-arm/page-shift.h
The name of the file looks a bit odd given that *_BITS are also defined
in it. So how about renaming to page-size.h?
I was initially meaning to use that name, but these headers
specifically don't define any sizes - *_BITS are still shift
values, at least in a way. If the current name isn't liked, my
next best suggestion would then be page-bits.h.
I would be happy with page-bits.h.
@@ -0,0 +1,15 @@
+#ifndef __ARM_PAGE_SHIFT_H__
+#define __ARM_PAGE_SHIFT_H__
+
+#define PAGE_SHIFT 12
+
+#define PAGE_OFFSET(ptr) ((vaddr_t)(ptr) & ~PAGE_MASK)
+
+#ifdef CONFIG_ARM_64
+#define PADDR_BITS 48
Shouldn't we define VADDR_BITS here?
See the description - it's unused anyway. I'm fine any of the three
possible ways:
1) keep as is in v1
2) drop altogether
3) also #define for 64-bit (but then you need to tell me whether 64
is the right value to use, or what the correct one would be)
I would go with 2).
But I wonder whether VADDR_BITS
should be defined as sizeof(vaddr_t) * 8.
This would require to include asm/types.h.
Which I'd specifically like to avoid. Plus use of sizeof() also
precludes the use of respective #define-s in #if-s.
Fair point!
--- a/xen/include/asm-x86/desc.h
+++ b/xen/include/asm-x86/desc.h
@@ -1,6 +1,8 @@
#ifndef __ARCH_DESC_H
#define __ARCH_DESC_H
+#include <asm/page.h>
May I ask why you are including <asm/page.h> and not <xen/page-size.h> here?
Because of
DECLARE_PER_CPU(l1_pgentry_t, gdt_l1e);
and
DECLARE_PER_CPU(l1_pgentry_t, compat_gdt_l1e);
at least (didn't check further).
Thanks for the explanation!
Jan
Cheers,
--
Julien Grall
