Message ID | 04276039-a5d0-fefd-260e-ffaa8272fd6a@suse.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | a tiny bit of header disentangling | expand |
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. > > Also drop the dead PAGE_FLAG_MASK altogether at this occasion. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- a/xen/arch/arm/arm64/lib/clear_page.S > +++ b/xen/arch/arm/arm64/lib/clear_page.S > @@ -14,6 +14,8 @@ > * along with this program. If not, see <http://www.gnu.org/licenses/>. > */ > > +#include <xen/page-size.h> > + > /* > * Clear page @dest > * > --- a/xen/include/asm-arm/config.h > +++ b/xen/include/asm-arm/config.h > @@ -176,11 +176,6 @@ > #define FIXMAP_ACPI_BEGIN 2 /* Start mappings of ACPI tables */ > #define FIXMAP_ACPI_END (FIXMAP_ACPI_BEGIN + NUM_FIXMAP_ACPI_PAGES - 1) /* End mappings of ACPI tables */ > > -#define PAGE_SHIFT 12 > -#define PAGE_SIZE (_AC(1,L) << PAGE_SHIFT) > -#define PAGE_MASK (~(PAGE_SIZE-1)) > -#define PAGE_FLAG_MASK (~0) > - > #define NR_hypercalls 64 > > #define STACK_ORDER 3 > --- a/xen/include/asm-arm/current.h > +++ b/xen/include/asm-arm/current.h > @@ -1,6 +1,7 @@ > #ifndef __ARM_CURRENT_H__ > #define __ARM_CURRENT_H__ > > +#include <xen/page-size.h> > #include <xen/percpu.h> > > #include <asm/processor.h> > --- /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? > @@ -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? But I wonder whether VADDR_BITS should be defined as sizeof(vaddr_t) * 8. This would require to include asm/types.h. > +#else > +#define PADDR_BITS 40 > +#define VADDR_BITS 32 > +#endif > + > +#endif /* __ARM_PAGE_SHIFT_H__ */ > --- a/xen/include/asm-arm/page.h > +++ b/xen/include/asm-arm/page.h > @@ -2,21 +2,11 @@ > #define __ARM_PAGE_H__ > > #include <public/xen.h> > +#include <xen/page-size.h> > #include <asm/processor.h> > #include <asm/lpae.h> > #include <asm/sysregs.h> > > -#ifdef CONFIG_ARM_64 > -#define PADDR_BITS 48 > -#else > -#define PADDR_BITS 40 > -#endif > -#define PADDR_MASK ((1ULL << PADDR_BITS)-1) > -#define PAGE_OFFSET(ptr) ((vaddr_t)(ptr) & ~PAGE_MASK) > - > -#define VADDR_BITS 32 > -#define VADDR_MASK (~0UL) > - > /* Shareability values for the LPAE entries */ > #define LPAE_SH_NON_SHAREABLE 0x0 > #define LPAE_SH_UNPREDICTALE 0x1 > --- a/xen/include/asm-x86/current.h > +++ b/xen/include/asm-x86/current.h > @@ -8,8 +8,8 @@ > #define __X86_CURRENT_H__ > > #include <xen/percpu.h> > +#include <xen/page-size.h> > #include <public/xen.h> > -#include <asm/page.h> > > /* > * Xen's cpu stacks are 8 pages (8-page aligned), arranged as: > --- 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? Cheers,
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. >> @@ -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) > 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. >> --- 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). Jan
On 03/12/2020 09:27, Jan Beulich wrote: >>> --- /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. Pick a generic name, or it will bitrot quickly, and it really wants to be the same across architectures. The real issue is that page.h contains far too much stuff now, in both architectures. Longterm, we want to split apart the architectural pagetable definitions, from the Xen APIs to manipulate them, which will simplify the asm include hierarchy as well. I'd go with page-bits.h, or just a plain pagetable.h. ~Andrew
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,
--- a/xen/arch/arm/arm64/lib/clear_page.S +++ b/xen/arch/arm/arm64/lib/clear_page.S @@ -14,6 +14,8 @@ * along with this program. If not, see <http://www.gnu.org/licenses/>. */ +#include <xen/page-size.h> + /* * Clear page @dest * --- a/xen/include/asm-arm/config.h +++ b/xen/include/asm-arm/config.h @@ -176,11 +176,6 @@ #define FIXMAP_ACPI_BEGIN 2 /* Start mappings of ACPI tables */ #define FIXMAP_ACPI_END (FIXMAP_ACPI_BEGIN + NUM_FIXMAP_ACPI_PAGES - 1) /* End mappings of ACPI tables */ -#define PAGE_SHIFT 12 -#define PAGE_SIZE (_AC(1,L) << PAGE_SHIFT) -#define PAGE_MASK (~(PAGE_SIZE-1)) -#define PAGE_FLAG_MASK (~0) - #define NR_hypercalls 64 #define STACK_ORDER 3 --- a/xen/include/asm-arm/current.h +++ b/xen/include/asm-arm/current.h @@ -1,6 +1,7 @@ #ifndef __ARM_CURRENT_H__ #define __ARM_CURRENT_H__ +#include <xen/page-size.h> #include <xen/percpu.h> #include <asm/processor.h> --- /dev/null +++ b/xen/include/asm-arm/page-shift.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 +#else +#define PADDR_BITS 40 +#define VADDR_BITS 32 +#endif + +#endif /* __ARM_PAGE_SHIFT_H__ */ --- a/xen/include/asm-arm/page.h +++ b/xen/include/asm-arm/page.h @@ -2,21 +2,11 @@ #define __ARM_PAGE_H__ #include <public/xen.h> +#include <xen/page-size.h> #include <asm/processor.h> #include <asm/lpae.h> #include <asm/sysregs.h> -#ifdef CONFIG_ARM_64 -#define PADDR_BITS 48 -#else -#define PADDR_BITS 40 -#endif -#define PADDR_MASK ((1ULL << PADDR_BITS)-1) -#define PAGE_OFFSET(ptr) ((vaddr_t)(ptr) & ~PAGE_MASK) - -#define VADDR_BITS 32 -#define VADDR_MASK (~0UL) - /* Shareability values for the LPAE entries */ #define LPAE_SH_NON_SHAREABLE 0x0 #define LPAE_SH_UNPREDICTALE 0x1 --- a/xen/include/asm-x86/current.h +++ b/xen/include/asm-x86/current.h @@ -8,8 +8,8 @@ #define __X86_CURRENT_H__ #include <xen/percpu.h> +#include <xen/page-size.h> #include <public/xen.h> -#include <asm/page.h> /* * Xen's cpu stacks are 8 pages (8-page aligned), arranged as: --- 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> + /* * Xen reserves a memory page of GDT entries. * No guest GDT entries exist beyond the Xen reserved area. --- a/xen/include/asm-x86/fixmap.h +++ b/xen/include/asm-x86/fixmap.h @@ -12,7 +12,7 @@ #ifndef _ASM_FIXMAP_H #define _ASM_FIXMAP_H -#include <asm/page.h> +#include <xen/page-size.h> #define FIXADDR_TOP (VMAP_VIRT_END - PAGE_SIZE) #define FIXADDR_X_TOP (XEN_VIRT_END - PAGE_SIZE) --- a/xen/include/asm-x86/guest/hyperv-hcall.h +++ b/xen/include/asm-x86/guest/hyperv-hcall.h @@ -20,12 +20,12 @@ #define __X86_HYPERV_HCALL_H__ #include <xen/lib.h> +#include <xen/page-size.h> #include <xen/types.h> #include <asm/asm_defns.h> #include <asm/fixmap.h> #include <asm/guest/hyperv-tlfs.h> -#include <asm/page.h> static inline uint64_t hv_do_hypercall(uint64_t control, paddr_t input_addr, paddr_t output_addr) --- a/xen/include/asm-x86/guest/hyperv-tlfs.h +++ b/xen/include/asm-x86/guest/hyperv-tlfs.h @@ -10,8 +10,8 @@ #define _ASM_X86_HYPERV_TLFS_H #include <xen/bitops.h> +#include <xen/page-size.h> #include <xen/types.h> -#include <asm/page.h> /* * While not explicitly listed in the TLFS, Hyper-V always runs with a page size --- a/xen/include/asm-x86/io.h +++ b/xen/include/asm-x86/io.h @@ -3,7 +3,6 @@ #include <xen/vmap.h> #include <xen/types.h> -#include <asm/page.h> #define readb(x) (*(volatile uint8_t *)(x)) #define readw(x) (*(volatile uint16_t *)(x)) --- a/xen/include/asm-x86/mm.h +++ b/xen/include/asm-x86/mm.h @@ -6,6 +6,7 @@ #include <xen/spinlock.h> #include <xen/rwlock.h> #include <asm/io.h> +#include <asm/page.h> #include <asm/uaccess.h> #include <asm/x86_emulate.h> --- /dev/null +++ b/xen/include/asm-x86/page-shift.h @@ -0,0 +1,26 @@ +#ifndef __X86_PAGE_SHIFT_H__ +#define __X86_PAGE_SHIFT_H__ + +#define L1_PAGETABLE_SHIFT 12 +#define L2_PAGETABLE_SHIFT 21 +#define L3_PAGETABLE_SHIFT 30 +#define L4_PAGETABLE_SHIFT 39 +#define PAGE_SHIFT L1_PAGETABLE_SHIFT +#define SUPERPAGE_SHIFT L2_PAGETABLE_SHIFT +#define ROOT_PAGETABLE_SHIFT L4_PAGETABLE_SHIFT + +#define PAGETABLE_ORDER 9 +#define L1_PAGETABLE_ENTRIES (1 << PAGETABLE_ORDER) +#define L2_PAGETABLE_ENTRIES (1 << PAGETABLE_ORDER) +#define L3_PAGETABLE_ENTRIES (1 << PAGETABLE_ORDER) +#define L4_PAGETABLE_ENTRIES (1 << PAGETABLE_ORDER) +#define ROOT_PAGETABLE_ENTRIES L4_PAGETABLE_ENTRIES + +#define SUPERPAGE_ORDER PAGETABLE_ORDER +#define SUPERPAGE_PAGES (1 << SUPERPAGE_ORDER) + +/* These are architectural limits. */ +#define PADDR_BITS 52 +#define VADDR_BITS 48 + +#endif /* __X86_PAGE_SHIFT_H__ */ --- a/xen/include/asm-x86/page.h +++ b/xen/include/asm-x86/page.h @@ -2,15 +2,7 @@ #define __X86_PAGE_H__ #include <xen/const.h> - -/* - * It is important that the masks are signed quantities. This ensures that - * the compiler sign-extends a 32-bit mask to 64 bits if that is required. - */ -#define PAGE_SIZE (_AC(1,L) << PAGE_SHIFT) -#define PAGE_MASK (~(PAGE_SIZE-1)) -#define PAGE_FLAG_MASK (~0) -#define PAGE_OFFSET(ptr) ((unsigned long)(ptr) & ~PAGE_MASK) +#include <xen/page-size.h> #define PAGE_ORDER_4K 0 #define PAGE_ORDER_2M 9 --- a/xen/include/asm-x86/uaccess.h +++ b/xen/include/asm-x86/uaccess.h @@ -6,7 +6,6 @@ #include <xen/errno.h> #include <xen/prefetch.h> #include <asm/asm_defns.h> -#include <asm/page.h> #include <asm/x86_64/uaccess.h> --- a/xen/include/asm-x86/x86_64/page.h +++ b/xen/include/asm-x86/x86_64/page.h @@ -2,31 +2,8 @@ #ifndef __X86_64_PAGE_H__ #define __X86_64_PAGE_H__ -#define L1_PAGETABLE_SHIFT 12 -#define L2_PAGETABLE_SHIFT 21 -#define L3_PAGETABLE_SHIFT 30 -#define L4_PAGETABLE_SHIFT 39 -#define PAGE_SHIFT L1_PAGETABLE_SHIFT -#define SUPERPAGE_SHIFT L2_PAGETABLE_SHIFT -#define ROOT_PAGETABLE_SHIFT L4_PAGETABLE_SHIFT - -#define PAGETABLE_ORDER 9 -#define L1_PAGETABLE_ENTRIES (1<<PAGETABLE_ORDER) -#define L2_PAGETABLE_ENTRIES (1<<PAGETABLE_ORDER) -#define L3_PAGETABLE_ENTRIES (1<<PAGETABLE_ORDER) -#define L4_PAGETABLE_ENTRIES (1<<PAGETABLE_ORDER) -#define ROOT_PAGETABLE_ENTRIES L4_PAGETABLE_ENTRIES -#define SUPERPAGE_ORDER PAGETABLE_ORDER -#define SUPERPAGE_PAGES (1<<SUPERPAGE_ORDER) - #define __XEN_VIRT_START XEN_VIRT_START -/* These are architectural limits. Current CPUs support only 40-bit phys. */ -#define PADDR_BITS 52 -#define VADDR_BITS 48 -#define PADDR_MASK ((_AC(1,UL) << PADDR_BITS) - 1) -#define VADDR_MASK ((_AC(1,UL) << VADDR_BITS) - 1) - #define VADDR_TOP_BIT (1UL << (VADDR_BITS - 1)) #define CANONICAL_MASK (~0UL & ~VADDR_MASK) --- a/xen/include/xen/gdbstub.h +++ b/xen/include/xen/gdbstub.h @@ -20,8 +20,8 @@ #ifndef __XEN_GDBSTUB_H__ #define __XEN_GDBSTUB_H__ +#include <xen/page-size.h> #include <asm/atomic.h> -#include <asm/page.h> #ifdef CONFIG_CRASH_DEBUG --- a/xen/include/xen/grant_table.h +++ b/xen/include/xen/grant_table.h @@ -26,7 +26,6 @@ #include <xen/mm.h> #include <xen/rwlock.h> #include <public/grant_table.h> -#include <asm/page.h> #include <asm/grant_table.h> #ifdef CONFIG_GRANT_TABLE --- /dev/null +++ b/xen/include/xen/page-size.h @@ -0,0 +1,21 @@ +#ifndef __XEN_PAGE_SIZE_H__ +#define __XEN_PAGE_SIZE_H__ + +#include <xen/const.h> +#include <asm/page-shift.h> + +/* + * It is important that the masks are signed quantities. This ensures that + * the compiler sign-extends a 32-bit mask to 64 bits if that is required. + */ +#define PAGE_SIZE (_AC(1,L) << PAGE_SHIFT) +#define PAGE_MASK (~(PAGE_SIZE-1)) + +#ifndef PAGE_OFFSET +# define PAGE_OFFSET(ptr) ((unsigned long)(ptr) & ~PAGE_MASK) +#endif + +#define PADDR_MASK ((_AC(1,ULL) << PADDR_BITS) - 1) +#define VADDR_MASK (~_AC(0,UL) >> (BITS_PER_LONG - VADDR_BITS)) + +#endif /* __XEN_PAGE_SIZE__ */ --- a/xen/include/xen/pfn.h +++ b/xen/include/xen/pfn.h @@ -1,7 +1,7 @@ #ifndef __XEN_PFN_H__ #define __XEN_PFN_H__ -#include <asm/page.h> +#include <xen/page-size.h> #define PFN_DOWN(x) ((x) >> PAGE_SHIFT) #define PFN_UP(x) (((x) + PAGE_SIZE-1) >> PAGE_SHIFT) --- a/xen/include/xen/vmap.h +++ b/xen/include/xen/vmap.h @@ -2,7 +2,7 @@ #define __XEN_VMAP_H__ #include <xen/mm.h> -#include <asm/page.h> +#include <xen/page-size.h> enum vmap_region { VMAP_DEFAULT,
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 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). 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. Also drop the dead PAGE_FLAG_MASK altogether at this occasion. Signed-off-by: Jan Beulich <jbeulich@suse.com>