| Message ID | 7ab8ce9ca633a5a7e5212d0acc62d6e1d4688ca0.1699633310.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce generic headers | expand |
This patch should be reworked as it fails Arm builds: https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/1068867920 It looks like it is not possible just to #ifdef CONFIG_MEM_ACCESS. An empty asm-generic mem_access.h will be better solution. Could you please share your opinion? ~ Oleksii On Fri, 2023-11-10 at 18:30 +0200, Oleksii Kurochko wrote: > ifdefing inclusion of <asm/mem_access.h> in <xen/mem_access.h> > allows to avoid generation of empty <asm/mem_access.h> header > for the case when !CONFIG_MEM_ACCESS. > > Suggested-by: Jan Beulich <jbeulich@suse.com> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> > --- > Changes in V2: > - add Suggested-by: Jan Beulich <jbeulich@suse.com> > - update the commit message > - remove <asm-generic/mem_access.h> > --- > xen/include/xen/mem_access.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/xen/include/xen/mem_access.h > b/xen/include/xen/mem_access.h > index 4e4811680d..87d93b31f6 100644 > --- a/xen/include/xen/mem_access.h > +++ b/xen/include/xen/mem_access.h > @@ -33,7 +33,9 @@ > */ > struct vm_event_st; > > +#ifdef CONFIG_MEM_ACCESS > #include <asm/mem_access.h> > +#endif > > /* > * Additional access types, which are used to further restrict
On 11.11.2023 11:24, Oleksii wrote: > This patch should be reworked as it fails Arm builds: > https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/1068867920 Took me a while to actually find the error. Would have been nice if you had explained what's going wrong, supplying the link only as extra info. > It looks like it is not possible just to #ifdef CONFIG_MEM_ACCESS. > > An empty asm-generic mem_access.h will be better solution. I remain unconvinced. The issue looks to be with p2m_mem_access_check(). One solution would be to move that declaration into the common header. But there's no common code referencing it, so Arm's and x86's version might as well diverge at some point. Hence from my pov it again boils down to Arm's traps.c including asm/mem_access.h explicitly, to bring the declaration in scope. None of the common files really have a need to have a dependency on the arch-specific header when MEM_ACCESS=n. Jan
On Mon, 2023-11-13 at 18:01 +0100, Jan Beulich wrote: > On 11.11.2023 11:24, Oleksii wrote: > > This patch should be reworked as it fails Arm builds: > > https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/1068867920 > > Took me a while to actually find the error. Would have been nice if > you > had explained what's going wrong, supplying the link only as extra > info. Sorry about that. I'll be more precise with links next time! But the issue is exactly what you wrote below ( with p2m_mem_access_check and some others from <asm/mem_access.h> ) . > > > It looks like it is not possible just to #ifdef CONFIG_MEM_ACCESS. > > > > An empty asm-generic mem_access.h will be better solution. > > I remain unconvinced. The issue looks to be with > p2m_mem_access_check(). > One solution would be to move that declaration into the common > header. > But there's no common code referencing it, so Arm's and x86's version > might as well diverge at some point. Hence from my pov it again boils > down to Arm's traps.c including asm/mem_access.h explicitly, to bring > the declaration in scope. None of the common files really have a need > to have a dependency on the arch-specific header when MEM_ACCESS=n. I started to do that in that way you mentioned. It should be included in 2 files, at least: p2m.c and traps.c. I am finishind the testing if it is enough. If it is enough, I will send a separate patch. ~ Oleksii
diff --git a/xen/include/xen/mem_access.h b/xen/include/xen/mem_access.h index 4e4811680d..87d93b31f6 100644 --- a/xen/include/xen/mem_access.h +++ b/xen/include/xen/mem_access.h @@ -33,7 +33,9 @@ */ struct vm_event_st; +#ifdef CONFIG_MEM_ACCESS #include <asm/mem_access.h> +#endif /* * Additional access types, which are used to further restrict
ifdefing inclusion of <asm/mem_access.h> in <xen/mem_access.h> allows to avoid generation of empty <asm/mem_access.h> header for the case when !CONFIG_MEM_ACCESS. Suggested-by: Jan Beulich <jbeulich@suse.com> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- Changes in V2: - add Suggested-by: Jan Beulich <jbeulich@suse.com> - update the commit message - remove <asm-generic/mem_access.h> --- xen/include/xen/mem_access.h | 2 ++ 1 file changed, 2 insertions(+)