| Message ID | 2702cfa486aa92e82fccd6393519073f10f4c40c.1691016993.git.sanastasio@raptorengineering.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ppc: Enable full Xen build | expand |
On 03.08.2023 01:03, Shawn Anastasio wrote: > --- a/xen/arch/ppc/mm-radix.c > +++ b/xen/arch/ppc/mm-radix.c > @@ -266,3 +266,47 @@ void __init setup_initial_pagetables(void) > /* Turn on the MMU */ > enable_mmu(); > } > + > + Nit: No double blank lines please. > +/* > + * TODO: Implement the functions below > + */ > +unsigned long total_pages; Hmm, yet one more prereq patch for me to make: There should be no need for every arch to have a definition, when common code requires the variable. While looking there I found max_page, which common code references as well. I'm surprised you get away without. I guess I'll learn why that is when making the patch moving both. > +unsigned long frametable_base_pdx __read_mostly; While we still have many instances like this, we prefer this form: unsigned long __read_mostly frametable_base_pdx; > + > +void put_page(struct page_info *p) > +{ > + BUG(); > +} > + > +void arch_dump_shared_mem_info(void) > +{ > + BUG(); > +} And perhaps one further prereq patch to avoid the need for this. > +int xenmem_add_to_physmap_one(struct domain *d, > + unsigned int space, > + union add_to_physmap_extra extra, > + unsigned long idx, > + gfn_t gfn) > +{ > + BUG(); > +} > + > +int destroy_xen_mappings(unsigned long s, unsigned long e) > +{ > + BUG(); > +} > + > +int map_pages_to_xen(unsigned long virt, > + mfn_t mfn, > + unsigned long nr_mfns, > + unsigned int flags) There's a patch in flight regarding the naming of this last parameter. I guess PPC would best be in sync right away. > --- a/xen/arch/ppc/setup.c > +++ b/xen/arch/ppc/setup.c > @@ -1,5 +1,8 @@ > /* SPDX-License-Identifier: GPL-2.0-or-later */ > +#include <xen/lib.h> > #include <xen/init.h> > +#include <xen/mm.h> > +#include <public/version.h> > #include <asm/boot.h> > #include <asm/early_printk.h> > #include <asm/processor.h> There's no need for xen/lib.h to come ahead of xen/init.h, is there? > --- /dev/null > +++ b/xen/arch/ppc/stubs.c > @@ -0,0 +1,351 @@ >[...] > +static void ack_none(struct irq_desc *irq) > +{ > + BUG(); > +} > + > +static void end_none(struct irq_desc *irq) > +{ > + BUG(); > +} > + > +hw_irq_controller no_irq_type = { > + .typename = "none", > + .startup = irq_startup_none, > + .shutdown = irq_shutdown_none, > + .enable = irq_enable_none, > + .disable = irq_disable_none, > + .ack = ack_none, > + .end = end_none > +}; I would recommend to avoid filling pointers (and hence having private hook functions) where it's not clear whether they'll be required. "end", for example, is an optional hook on x86. Iirc common code doesn't use any of the hooks. Jan
On 8/8/23 5:27 AM, Jan Beulich wrote: > On 03.08.2023 01:03, Shawn Anastasio wrote: >> --- a/xen/arch/ppc/mm-radix.c >> +++ b/xen/arch/ppc/mm-radix.c >> @@ -266,3 +266,47 @@ void __init setup_initial_pagetables(void) >> /* Turn on the MMU */ >> enable_mmu(); >> } >> + >> + > > Nit: No double blank lines please. > Will fix. >> +/* >> + * TODO: Implement the functions below >> + */ >> +unsigned long total_pages; > > Hmm, yet one more prereq patch for me to make: There should be no need > for every arch to have a definition, when common code requires the > variable. While looking there I found max_page, which common code > references as well. I'm surprised you get away without. I guess I'll > learn why that is when making the patch moving both. > Since your patch for this has gone though, I'll drop this in v2. >> +unsigned long frametable_base_pdx __read_mostly; > > While we still have many instances like this, we prefer this form: > > unsigned long __read_mostly frametable_base_pdx; > Will update. >> + >> +void put_page(struct page_info *p) >> +{ >> + BUG(); >> +} >> + >> +void arch_dump_shared_mem_info(void) >> +{ >> + BUG(); >> +} > > And perhaps one further prereq patch to avoid the need for this. > Will remove from this series pending merge of your prereq patch. >> +int xenmem_add_to_physmap_one(struct domain *d, >> + unsigned int space, >> + union add_to_physmap_extra extra, >> + unsigned long idx, >> + gfn_t gfn) >> +{ >> + BUG(); >> +} >> + >> +int destroy_xen_mappings(unsigned long s, unsigned long e) >> +{ >> + BUG(); >> +} >> + >> +int map_pages_to_xen(unsigned long virt, >> + mfn_t mfn, >> + unsigned long nr_mfns, >> + unsigned int flags) > > There's a patch in flight regarding the naming of this last parameter. > I guess PPC would best be in sync right away. > I can't seem to find the patch in question and it doesn't seem like it has been merged in the meantime. Could you provide a link? >> --- a/xen/arch/ppc/setup.c >> +++ b/xen/arch/ppc/setup.c >> @@ -1,5 +1,8 @@ >> /* SPDX-License-Identifier: GPL-2.0-or-later */ >> +#include <xen/lib.h> >> #include <xen/init.h> >> +#include <xen/mm.h> >> +#include <public/version.h> >> #include <asm/boot.h> >> #include <asm/early_printk.h> >> #include <asm/processor.h> > > There's no need for xen/lib.h to come ahead of xen/init.h, is there? > There is not -- I'll fix the ordering. >> --- /dev/null >> +++ b/xen/arch/ppc/stubs.c >> @@ -0,0 +1,351 @@ >> [...] >> +static void ack_none(struct irq_desc *irq) >> +{ >> + BUG(); >> +} >> + >> +static void end_none(struct irq_desc *irq) >> +{ >> + BUG(); >> +} >> + >> +hw_irq_controller no_irq_type = { >> + .typename = "none", >> + .startup = irq_startup_none, >> + .shutdown = irq_shutdown_none, >> + .enable = irq_enable_none, >> + .disable = irq_disable_none, >> + .ack = ack_none, >> + .end = end_none >> +}; > > I would recommend to avoid filling pointers (and hence having private > hook functions) where it's not clear whether they'll be required. "end", > for example, is an optional hook on x86. Iirc common code doesn't use > any of the hooks. > Alright, I'll drop the `end_none` stub and leave the .end pointer as NULL. > Jan Thanks, Shawn
On 23.08.2023 20:39, Shawn Anastasio wrote: > On 8/8/23 5:27 AM, Jan Beulich wrote: >> On 03.08.2023 01:03, Shawn Anastasio wrote: >>> +int map_pages_to_xen(unsigned long virt, >>> + mfn_t mfn, >>> + unsigned long nr_mfns, >>> + unsigned int flags) >> >> There's a patch in flight regarding the naming of this last parameter. >> I guess PPC would best be in sync right away. >> > > I can't seem to find the patch in question and it doesn't seem like it > has been merged in the meantime. Could you provide a link? Looks like I was misremembering, and it was modify_xen_mappings() instead. I'm sorry for the noise. >>> --- /dev/null >>> +++ b/xen/arch/ppc/stubs.c >>> @@ -0,0 +1,351 @@ >>> [...] >>> +static void ack_none(struct irq_desc *irq) >>> +{ >>> + BUG(); >>> +} >>> + >>> +static void end_none(struct irq_desc *irq) >>> +{ >>> + BUG(); >>> +} >>> + >>> +hw_irq_controller no_irq_type = { >>> + .typename = "none", >>> + .startup = irq_startup_none, >>> + .shutdown = irq_shutdown_none, >>> + .enable = irq_enable_none, >>> + .disable = irq_disable_none, >>> + .ack = ack_none, >>> + .end = end_none >>> +}; >> >> I would recommend to avoid filling pointers (and hence having private >> hook functions) where it's not clear whether they'll be required. "end", >> for example, is an optional hook on x86. Iirc common code doesn't use >> any of the hooks. > > Alright, I'll drop the `end_none` stub and leave the .end pointer as > NULL. Yet my comment was about all the (presently dead) hook functions. Jan
On 8/25/23 4:10 AM, Jan Beulich wrote: > On 23.08.2023 20:39, Shawn Anastasio wrote: >> On 8/8/23 5:27 AM, Jan Beulich wrote: >>> On 03.08.2023 01:03, Shawn Anastasio wrote: >>>> +int map_pages_to_xen(unsigned long virt, >>>> + mfn_t mfn, >>>> + unsigned long nr_mfns, >>>> + unsigned int flags) >>> >>> There's a patch in flight regarding the naming of this last parameter. >>> I guess PPC would best be in sync right away. >>> >> >> I can't seem to find the patch in question and it doesn't seem like it >> has been merged in the meantime. Could you provide a link? > > Looks like I was misremembering, and it was modify_xen_mappings() instead. > I'm sorry for the noise. > No problem. >>>> --- /dev/null >>>> +++ b/xen/arch/ppc/stubs.c >>>> @@ -0,0 +1,351 @@ >>>> [...] >>>> +static void ack_none(struct irq_desc *irq) >>>> +{ >>>> + BUG(); >>>> +} >>>> + >>>> +static void end_none(struct irq_desc *irq) >>>> +{ >>>> + BUG(); >>>> +} >>>> + >>>> +hw_irq_controller no_irq_type = { >>>> + .typename = "none", >>>> + .startup = irq_startup_none, >>>> + .shutdown = irq_shutdown_none, >>>> + .enable = irq_enable_none, >>>> + .disable = irq_disable_none, >>>> + .ack = ack_none, >>>> + .end = end_none >>>> +}; >>> >>> I would recommend to avoid filling pointers (and hence having private >>> hook functions) where it's not clear whether they'll be required. "end", >>> for example, is an optional hook on x86. Iirc common code doesn't use >>> any of the hooks. >> >> Alright, I'll drop the `end_none` stub and leave the .end pointer as >> NULL. > > Yet my comment was about all the (presently dead) hook functions. Sorry, I misunderstood. To clarify, by "hook functions" you're referring to all of the function pointer fields of hw_irq_controller? Are all users of this struct going to properly check for NULL before trying to call these function pointers? > Jan Thanks, Shawn
On 25.08.2023 19:26, Shawn Anastasio wrote: > On 8/25/23 4:10 AM, Jan Beulich wrote: >> On 23.08.2023 20:39, Shawn Anastasio wrote: >>> On 8/8/23 5:27 AM, Jan Beulich wrote: >>>> On 03.08.2023 01:03, Shawn Anastasio wrote: >>>>> --- /dev/null >>>>> +++ b/xen/arch/ppc/stubs.c >>>>> @@ -0,0 +1,351 @@ >>>>> [...] >>>>> +static void ack_none(struct irq_desc *irq) >>>>> +{ >>>>> + BUG(); >>>>> +} >>>>> + >>>>> +static void end_none(struct irq_desc *irq) >>>>> +{ >>>>> + BUG(); >>>>> +} >>>>> + >>>>> +hw_irq_controller no_irq_type = { >>>>> + .typename = "none", >>>>> + .startup = irq_startup_none, >>>>> + .shutdown = irq_shutdown_none, >>>>> + .enable = irq_enable_none, >>>>> + .disable = irq_disable_none, >>>>> + .ack = ack_none, >>>>> + .end = end_none >>>>> +}; >>>> >>>> I would recommend to avoid filling pointers (and hence having private >>>> hook functions) where it's not clear whether they'll be required. "end", >>>> for example, is an optional hook on x86. Iirc common code doesn't use >>>> any of the hooks. >>> >>> Alright, I'll drop the `end_none` stub and leave the .end pointer as >>> NULL. >> >> Yet my comment was about all the (presently dead) hook functions. > > Sorry, I misunderstood. To clarify, by "hook functions" you're referring > to all of the function pointer fields of hw_irq_controller? Are all > users of this struct going to properly check for NULL before trying to > call these function pointers? If I'm not mistaken, all uses of these hooks are in arch-specific code (we don't have a proper common layer, unlike Linux). Hence what checks there are going to be is all up to you. But as long as none of the fields are used, supplying (stub) hooks is pointless in the first place. IOW you can leave the pointers at NULL now and still use them later without checks, so long as at _that_ time you populate them (and then not just pointing to stub functions). My main point here is: This introduction of temporary stubs would better be as little overhead and as little code churn as possible - minimum number of new symbols/functions/objects, just enough for things to build. Jan
diff --git a/xen/arch/ppc/Makefile b/xen/arch/ppc/Makefile index a059ac4c0a..969910b3b6 100644 --- a/xen/arch/ppc/Makefile +++ b/xen/arch/ppc/Makefile @@ -5,6 +5,7 @@ obj-$(CONFIG_EARLY_PRINTK) += early_printk.init.o obj-y += mm-radix.o obj-y += opal.o obj-y += setup.o +obj-y += stubs.o obj-y += tlb-radix.o $(TARGET): $(TARGET)-syms diff --git a/xen/arch/ppc/mm-radix.c b/xen/arch/ppc/mm-radix.c index 399898a36d..1ed6897366 100644 --- a/xen/arch/ppc/mm-radix.c +++ b/xen/arch/ppc/mm-radix.c @@ -266,3 +266,47 @@ void __init setup_initial_pagetables(void) /* Turn on the MMU */ enable_mmu(); } + + +/* + * TODO: Implement the functions below + */ +unsigned long total_pages; +unsigned long frametable_base_pdx __read_mostly; + +void put_page(struct page_info *p) +{ + BUG(); +} + +void arch_dump_shared_mem_info(void) +{ + BUG(); +} + +int xenmem_add_to_physmap_one(struct domain *d, + unsigned int space, + union add_to_physmap_extra extra, + unsigned long idx, + gfn_t gfn) +{ + BUG(); +} + +int destroy_xen_mappings(unsigned long s, unsigned long e) +{ + BUG(); +} + +int map_pages_to_xen(unsigned long virt, + mfn_t mfn, + unsigned long nr_mfns, + unsigned int flags) +{ + BUG(); +} + +int __init populate_pt_range(unsigned long virt, unsigned long nr_mfns) +{ + BUG(); +} diff --git a/xen/arch/ppc/setup.c b/xen/arch/ppc/setup.c index 466993987b..06c0a5fa80 100644 --- a/xen/arch/ppc/setup.c +++ b/xen/arch/ppc/setup.c @@ -1,5 +1,8 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */ +#include <xen/lib.h> #include <xen/init.h> +#include <xen/mm.h> +#include <public/version.h> #include <asm/boot.h> #include <asm/early_printk.h> #include <asm/processor.h> @@ -7,8 +10,6 @@ /* Xen stack for bringing up the first CPU. */ unsigned char __initdata cpu0_boot_stack[STACK_SIZE] __aligned(STACK_SIZE); -void __init setup_initial_pagetables(void); - void __init noreturn start_xen(unsigned long r3, unsigned long r4, unsigned long r5, unsigned long r6, unsigned long r7) @@ -39,3 +40,8 @@ void __init noreturn start_xen(unsigned long r3, unsigned long r4, unreachable(); } + +void arch_get_xen_caps(xen_capabilities_info_t *info) +{ + BUG(); +} diff --git a/xen/arch/ppc/stubs.c b/xen/arch/ppc/stubs.c new file mode 100644 index 0000000000..2b3ee94115 --- /dev/null +++ b/xen/arch/ppc/stubs.c @@ -0,0 +1,351 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#include <xen/cpumask.h> +#include <xen/domain.h> +#include <xen/irq.h> +#include <xen/nodemask.h> +#include <xen/time.h> +#include <public/domctl.h> +#include <public/vm_event.h> + +#include <asm/current.h> + +/* smpboot.c */ + +cpumask_t cpu_online_map; +cpumask_t cpu_present_map; +cpumask_t cpu_possible_map; + +/* ID of the PCPU we're running on */ +DEFINE_PER_CPU(unsigned int, cpu_id); +/* XXX these seem awfully x86ish... */ +/* representing HT siblings of each logical CPU */ +DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_sibling_mask); +/* representing HT and core siblings of each logical CPU */ +DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_core_mask); + +nodemask_t __read_mostly node_online_map = { { [0] = 1UL } }; + +/* time.c */ + +s_time_t get_s_time(void) +{ + BUG(); +} + +int reprogram_timer(s_time_t timeout) +{ + BUG(); +} + +void send_timer_event(struct vcpu *v) +{ + BUG(); +} + +/* traps.c */ + +void show_execution_state(const struct cpu_user_regs *regs) +{ + BUG(); +} + +void arch_hypercall_tasklet_result(struct vcpu *v, long res) +{ + BUG(); +} + +void vcpu_show_execution_state(struct vcpu *v) +{ + BUG(); +} + +/* shutdown.c */ + +void machine_restart(unsigned int delay_millisecs) +{ + BUG(); +} + +void machine_halt(void) +{ + BUG(); +} + +/* vm_event.c */ + +void vm_event_fill_regs(vm_event_request_t *req) +{ + BUG(); +} + +void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp) +{ + BUG(); +} + +void vm_event_monitor_next_interrupt(struct vcpu *v) +{ + /* Not supported on PPC. */ +} + +/* domctl.c */ +void arch_get_domain_info(const struct domain *d, + struct xen_domctl_getdomaininfo *info) +{ + BUG(); +} + +/* monitor.c */ + +int arch_monitor_domctl_event(struct domain *d, + struct xen_domctl_monitor_op *mop) +{ + BUG(); +} + +/* smp.c */ + +void arch_flush_tlb_mask(const cpumask_t *mask) +{ + BUG(); +} + +void smp_send_event_check_mask(const cpumask_t *mask) +{ + BUG(); +} + +void smp_send_call_function_mask(const cpumask_t *mask) +{ + BUG(); +} + +/* irq.c */ + +struct pirq *alloc_pirq_struct(struct domain *d) +{ + BUG(); +} + +int pirq_guest_bind(struct vcpu *v, struct pirq *pirq, int will_share) +{ + BUG(); +} + +void pirq_guest_unbind(struct domain *d, struct pirq *pirq) +{ + BUG(); +} + +void pirq_set_affinity(struct domain *d, int pirq, const cpumask_t *mask) +{ + BUG(); +} + +static void ack_none(struct irq_desc *irq) +{ + BUG(); +} + +static void end_none(struct irq_desc *irq) +{ + BUG(); +} + +hw_irq_controller no_irq_type = { + .typename = "none", + .startup = irq_startup_none, + .shutdown = irq_shutdown_none, + .enable = irq_enable_none, + .disable = irq_disable_none, + .ack = ack_none, + .end = end_none +}; + +int arch_init_one_irq_desc(struct irq_desc *desc) +{ + BUG(); +} + +void smp_send_state_dump(unsigned int cpu) +{ + BUG(); +} + +/* domain.c */ + +DEFINE_PER_CPU(struct vcpu *, curr_vcpu); +unsigned long __per_cpu_offset[NR_CPUS]; + +void context_switch(struct vcpu *prev, struct vcpu *next) +{ + BUG(); +} + +void continue_running(struct vcpu *same) +{ + BUG(); +} + +void sync_local_execstate(void) +{ + BUG(); +} + +void sync_vcpu_execstate(struct vcpu *v) +{ + BUG(); +} + +void startup_cpu_idle_loop(void) +{ + BUG(); +} + +void free_domain_struct(struct domain *d) +{ + BUG(); +} + +void dump_pageframe_info(struct domain *d) +{ + BUG(); +} + +void free_vcpu_struct(struct vcpu *v) +{ + BUG(); +} + +int arch_vcpu_create(struct vcpu *v) +{ + BUG(); +} + +void arch_vcpu_destroy(struct vcpu *v) +{ + BUG(); +} + +void vcpu_switch_to_aarch64_mode(struct vcpu *v) +{ + BUG(); +} + +int arch_sanitise_domain_config(struct xen_domctl_createdomain *config) +{ + BUG(); +} + +int arch_domain_create(struct domain *d, + struct xen_domctl_createdomain *config, + unsigned int flags) +{ + BUG(); +} + +int arch_domain_teardown(struct domain *d) +{ + BUG(); +} + +void arch_domain_destroy(struct domain *d) +{ + BUG(); +} + +void arch_domain_shutdown(struct domain *d) +{ + BUG(); +} + +void arch_domain_pause(struct domain *d) +{ + BUG(); +} + +void arch_domain_unpause(struct domain *d) +{ + BUG(); +} + +int arch_domain_soft_reset(struct domain *d) +{ + BUG(); +} + +void arch_domain_creation_finished(struct domain *d) +{ + BUG(); +} + +int arch_set_info_guest(struct vcpu *v, vcpu_guest_context_u c) +{ + BUG(); +} + +int arch_initialise_vcpu(struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg) +{ + BUG(); +} + +int arch_vcpu_reset(struct vcpu *v) +{ + BUG(); +} + +int domain_relinquish_resources(struct domain *d) +{ + BUG(); +} + +void arch_dump_domain_info(struct domain *d) +{ + BUG(); +} + +void arch_dump_vcpu_info(struct vcpu *v) +{ + BUG(); +} + +void vcpu_mark_events_pending(struct vcpu *v) +{ + BUG(); +} + +void vcpu_update_evtchn_irq(struct vcpu *v) +{ + BUG(); +} + +void vcpu_block_unless_event_pending(struct vcpu *v) +{ + BUG(); +} + +void vcpu_kick(struct vcpu *v) +{ + BUG(); +} + +struct domain *alloc_domain_struct(void) +{ + BUG(); +} + +struct vcpu *alloc_vcpu_struct(const struct domain *d) +{ + BUG(); +} + +unsigned long +hypercall_create_continuation(unsigned int op, const char *format, ...) +{ + BUG(); +} + +int __init parse_arch_dom0_param(const char *s, const char *e) +{ + BUG(); +}
Add stub function and symbol definitions required by common code. If the file that the definition is supposed to be located in doesn't already exist yet, temporarily place its definition in the new stubs.c Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com> --- xen/arch/ppc/Makefile | 1 + xen/arch/ppc/mm-radix.c | 44 +++++ xen/arch/ppc/setup.c | 10 +- xen/arch/ppc/stubs.c | 351 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 404 insertions(+), 2 deletions(-) create mode 100644 xen/arch/ppc/stubs.c