Re: [PATCH V3 14/23] arm/ioreq: Introduce arch specific bits for IOREQ/DM features

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 10 Dec 2020, Oleksandr wrote:
> > > +#ifdef CONFIG_IOREQ_SERVER
> > > +enum io_state handle_ioserv(struct cpu_user_regs *regs, struct vcpu *v);
> > > +enum io_state try_fwd_ioserv(struct cpu_user_regs *regs,
> > > +                             struct vcpu *v, mmio_info_t *info);
> > > +#else
> > > +static inline enum io_state handle_ioserv(struct cpu_user_regs *regs,
> > > +                                          struct vcpu *v)
> > > +{
> > > +    return IO_UNHANDLED;
> > > +}
> > > +
> > > +static inline enum io_state try_fwd_ioserv(struct cpu_user_regs *regs,
> > > +                                           struct vcpu *v, mmio_info_t
> > > *info)
> > > +{
> > > +    return IO_UNHANDLED;
> > > +}
> > > +#endif
> > If we are providing stub functions, then we can also provide stub
> > functions for:
> > 
> > ioreq_domain_init
> > ioreq_server_destroy_all
> > 
> > and avoid the ifdefs.
> I got your point. These are common IOREQ interface functions, which
> declarations live in the common header, should I provide
> stubs in the common ioreq.h?
 
I'd prefer that, but if Jan and Paul don't want to have them I won't insist.
 
 
> > > +bool ioreq_complete_mmio(void);
> > > +
> > > +static inline bool handle_pio(uint16_t port, unsigned int size, int dir)
> > > +{
> > > +    /*
> > > +     * TODO: For Arm64, the main user will be PCI. So this should be
> > > +     * implemented when we add support for vPCI.
> > > +     */
> > > +    ASSERT_UNREACHABLE();
> > > +    return true;
> > > +}
> > > +
> > > +static inline void msix_write_completion(struct vcpu *v)
> > > +{
> > > +}
> > > +
> > > +static inline bool arch_vcpu_ioreq_completion(enum io_completion
> > > io_completion)
> > > +{
> > > +    ASSERT_UNREACHABLE();
> > > +    return true;
> > > +}
> > > +
> > > +/*
> > > + * The "legacy" mechanism of mapping magic pages for the IOREQ servers
> > > + * is x86 specific, so the following hooks don't need to be implemented
> > > on Arm:
> > > + * - arch_ioreq_server_map_pages
> > > + * - arch_ioreq_server_unmap_pages
> > > + * - arch_ioreq_server_enable
> > > + * - arch_ioreq_server_disable
> > > + */
> > > +static inline int arch_ioreq_server_map_pages(struct ioreq_server *s)
> > > +{
> > > +    return -EOPNOTSUPP;
> > > +}
> > > +
> > > +static inline void arch_ioreq_server_unmap_pages(struct ioreq_server *s)
> > > +{
> > > +}
> > > +
> > > +static inline void arch_ioreq_server_enable(struct ioreq_server *s)
> > > +{
> > > +}
> > > +
> > > +static inline void arch_ioreq_server_disable(struct ioreq_server *s)
> > > +{
> > > +}
> > > +
> > > +static inline void arch_ioreq_server_destroy(struct ioreq_server *s)
> > > +{
> > > +}
> > > +
> > > +static inline int arch_ioreq_server_map_mem_type(struct domain *d,
> > > +                                                 struct ioreq_server *s,
> > > +                                                 uint32_t flags)
> > > +{
> > > +    return -EOPNOTSUPP;
> > > +}
> > > +
> > > +static inline bool arch_ioreq_server_destroy_all(struct domain *d)
> > > +{
> > > +    return true;
> > > +}
> > > +
> > > +static inline int arch_ioreq_server_get_type_addr(const struct domain *d,
> > > +                                                  const ioreq_t *p,
> > > +                                                  uint8_t *type,
> > > +                                                  uint64_t *addr)
> > > +{
> > > +    if ( p->type != IOREQ_TYPE_COPY && p->type != IOREQ_TYPE_PIO )
> > > +        return -EINVAL;
> > > +
> > > +    *type = (p->type == IOREQ_TYPE_PIO) ?
> > > +             XEN_DMOP_IO_RANGE_PORT : XEN_DMOP_IO_RANGE_MEMORY;
> > > +    *addr = p->addr;
> > This function is not used in this patch and PIOs are left unimplemented
> > according to a few comments, so I am puzzled by this code here. Do we
> > need it?
> Yes. It is called from ioreq_server_select (common/ioreq.c). I could just skip
> PIO case and use
> *type = XEN_DMOP_IO_RANGE_MEMORY, but I didn't want to diverge.
 
I see. OK then.