diff mbox series

[V4,11/24] xen/mm: Make x86's XENMEM_resource_ioreq_server handling common

Message ID 1610488352-18494-12-git-send-email-olekstysh@gmail.com (mailing list archive)
State Superseded
Headers show
Series IOREQ feature (+ virtio-mmio) on Arm | expand

Commit Message

Oleksandr Tyshchenko Jan. 12, 2021, 9:52 p.m. UTC
From: Julien Grall <julien.grall@arm.com>

As x86 implementation of XENMEM_resource_ioreq_server can be
re-used on Arm later on, this patch makes it common and removes
arch_acquire_resource as unneeded.

Also re-order #include-s alphabetically.

This support is going to be used on Arm to be able run device
emulator outside of Xen hypervisor.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
[On Arm only]
Tested-by: Wei Chen <Wei.Chen@arm.com>

---
Please note, this is a split/cleanup/hardening of Julien's PoC:
"Add support for Guest IO forwarding to a device emulator"

Changes RFC -> V1:
   - no changes

Changes V1 -> V2:
   - update the author of a patch

Changes V2 -> V3:
   - don't wrap #include <xen/ioreq.h>
   - limit the number of #ifdef-s
   - re-order #include-s alphabetically

Changes V3 -> V4:
   - rebase
   - Add Jan's R-b
---
 xen/arch/x86/mm.c        | 44 ---------------------------------
 xen/common/memory.c      | 63 +++++++++++++++++++++++++++++++++++++++---------
 xen/include/asm-arm/mm.h |  8 ------
 xen/include/asm-x86/mm.h |  4 ---
 4 files changed, 51 insertions(+), 68 deletions(-)

Comments

Wei Chen Jan. 14, 2021, 3:58 a.m. UTC | #1
Hi Oleksandr,

> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of
> Oleksandr Tyshchenko
> Sent: 2021年1月13日 5:52
> To: xen-devel@lists.xenproject.org
> Cc: Julien Grall <Julien.Grall@arm.com>; Jan Beulich <jbeulich@suse.com>;
> Andrew Cooper <andrew.cooper3@citrix.com>; Roger Pau Monné
> <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; George Dunlap
> <george.dunlap@citrix.com>; Ian Jackson <iwj@xenproject.org>; Julien Grall
> <julien@xen.org>; Stefano Stabellini <sstabellini@kernel.org>; Volodymyr
> Babchuk <Volodymyr_Babchuk@epam.com>; Oleksandr Tyshchenko
> <oleksandr_tyshchenko@epam.com>
> Subject: [PATCH V4 11/24] xen/mm: Make x86's
> XENMEM_resource_ioreq_server handling common
> 
> From: Julien Grall <julien.grall@arm.com>
> 
> As x86 implementation of XENMEM_resource_ioreq_server can be
> re-used on Arm later on, this patch makes it common and removes
> arch_acquire_resource as unneeded.
> 
> Also re-order #include-s alphabetically.
> 
> This support is going to be used on Arm to be able run device
> emulator outside of Xen hypervisor.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> [On Arm only]
> Tested-by: Wei Chen <Wei.Chen@arm.com>
> 
> ---
> Please note, this is a split/cleanup/hardening of Julien's PoC:
> "Add support for Guest IO forwarding to a device emulator"
> 
> Changes RFC -> V1:
>    - no changes
> 
> Changes V1 -> V2:
>    - update the author of a patch
> 
> Changes V2 -> V3:
>    - don't wrap #include <xen/ioreq.h>
>    - limit the number of #ifdef-s
>    - re-order #include-s alphabetically
> 
> Changes V3 -> V4:
>    - rebase
>    - Add Jan's R-b
> ---
>  xen/arch/x86/mm.c        | 44 ---------------------------------
>  xen/common/memory.c      | 63
> +++++++++++++++++++++++++++++++++++++++---------
>  xen/include/asm-arm/mm.h |  8 ------
>  xen/include/asm-x86/mm.h |  4 ---
>  4 files changed, 51 insertions(+), 68 deletions(-)
> 
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index f6e128e..54ac398 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -4587,50 +4587,6 @@ static int handle_iomem_range(unsigned long s,
> unsigned long e, void *p)
>      return err || s > e ? err : _handle_iomem_range(s, e, p);
>  }
> 
> -int arch_acquire_resource(struct domain *d, unsigned int type,
> -                          unsigned int id, unsigned long frame,
> -                          unsigned int nr_frames, xen_pfn_t mfn_list[])
> -{
> -    int rc;
> -
> -    switch ( type )
> -    {
> -#ifdef CONFIG_HVM
> -    case XENMEM_resource_ioreq_server:
> -    {
> -        ioservid_t ioservid = id;
> -        unsigned int i;
> -
> -        rc = -EINVAL;
> -        if ( !is_hvm_domain(d) )
> -            break;
> -
> -        if ( id != (unsigned int)ioservid )
> -            break;
> -
> -        rc = 0;
> -        for ( i = 0; i < nr_frames; i++ )
> -        {
> -            mfn_t mfn;
> -
> -            rc = hvm_get_ioreq_server_frame(d, id, frame + i, &mfn);
> -            if ( rc )
> -                break;
> -
> -            mfn_list[i] = mfn_x(mfn);
> -        }
> -        break;
> -    }
> -#endif
> -
> -    default:
> -        rc = -EOPNOTSUPP;
> -        break;
> -    }
> -
> -    return rc;
> -}
> -
>  long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void)
> arg)
>  {
>      int rc;
> diff --git a/xen/common/memory.c b/xen/common/memory.c
> index b21b6c4..7e560b5 100644
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -8,22 +8,23 @@
>   */
> 
>  #include <xen/domain_page.h>
> -#include <xen/types.h>
> +#include <xen/errno.h>
> +#include <xen/event.h>
> +#include <xen/grant_table.h>
> +#include <xen/guest_access.h>
> +#include <xen/hypercall.h>
> +#include <xen/iocap.h>
> +#include <xen/ioreq.h>
>  #include <xen/lib.h>
> +#include <xen/mem_access.h>
>  #include <xen/mm.h>
> +#include <xen/numa.h>
> +#include <xen/paging.h>
>  #include <xen/param.h>
>  #include <xen/perfc.h>
>  #include <xen/sched.h>
> -#include <xen/event.h>
> -#include <xen/paging.h>
> -#include <xen/iocap.h>
> -#include <xen/guest_access.h>
> -#include <xen/hypercall.h>
> -#include <xen/errno.h>
> -#include <xen/numa.h>
> -#include <xen/mem_access.h>
>  #include <xen/trace.h>
> -#include <xen/grant_table.h>
> +#include <xen/types.h>
>  #include <asm/current.h>
>  #include <asm/hardirq.h>
>  #include <asm/p2m.h>
> @@ -1090,6 +1091,40 @@ static int acquire_grant_table(struct domain *d,
> unsigned int id,
>      return 0;
>  }
> 
> +static int acquire_ioreq_server(struct domain *d,
> +                                unsigned int id,
> +                                unsigned long frame,
> +                                unsigned int nr_frames,
> +                                xen_pfn_t mfn_list[])
> +{
> +#ifdef CONFIG_IOREQ_SERVER
> +    ioservid_t ioservid = id;
> +    unsigned int i;
> +    int rc;
> +
> +    if ( !is_hvm_domain(d) )
> +        return -EINVAL;
> +
> +    if ( id != (unsigned int)ioservid )
> +        return -EINVAL;
> +
> +    for ( i = 0; i < nr_frames; i++ )
> +    {
> +        mfn_t mfn;
> +
> +        rc = hvm_get_ioreq_server_frame(d, id, frame + i, &mfn);
> +        if ( rc )
> +            return rc;
> +
> +        mfn_list[i] = mfn_x(mfn);
> +    }
> +
> +    return 0;
> +#else
> +    return -EOPNOTSUPP;
> +#endif
> +}
> +
>  static int acquire_resource(
>      XEN_GUEST_HANDLE_PARAM(xen_mem_acquire_resource_t) arg)
>  {
> @@ -1148,9 +1183,13 @@ static int acquire_resource(
>                                   mfn_list);
>          break;
> 
> +    case XENMEM_resource_ioreq_server:
> +        rc = acquire_ioreq_server(d, xmar.id, xmar.frame, xmar.nr_frames,
> +                                  mfn_list);
> +        break;
> +
>      default:
> -        rc = arch_acquire_resource(d, xmar.type, xmar.id, xmar.frame,
> -                                   xmar.nr_frames, mfn_list);
> +        rc = -EOPNOTSUPP;
>          break;
>      }
> 
> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> index f8ba49b..0b7de31 100644
> --- a/xen/include/asm-arm/mm.h
> +++ b/xen/include/asm-arm/mm.h
> @@ -358,14 +358,6 @@ static inline void put_page_and_type(struct page_info
> *page)
> 
>  void clear_and_clean_page(struct page_info *page);
> 
> -static inline
> -int arch_acquire_resource(struct domain *d, unsigned int type, unsigned int id,
> -                          unsigned long frame, unsigned int nr_frames,
> -                          xen_pfn_t mfn_list[])
> -{
> -    return -EOPNOTSUPP;
> -}
> -
>  unsigned int arch_get_dma_bitsize(void);
> 

This change could not be applied to the latest staging branch.

>  #endif /*  __ARCH_ARM_MM__ */
> diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h
> index deeba75..859214e 100644
> --- a/xen/include/asm-x86/mm.h
> +++ b/xen/include/asm-x86/mm.h
> @@ -639,8 +639,4 @@ static inline bool arch_mfn_in_directmap(unsigned long
> mfn)
>      return mfn <= (virt_to_mfn(eva - 1) + 1);
>  }
> 
> -int arch_acquire_resource(struct domain *d, unsigned int type,
> -                          unsigned int id, unsigned long frame,
> -                          unsigned int nr_frames, xen_pfn_t mfn_list[]);
> -
>  #endif /* __ASM_X86_MM_H__ */
> --
> 2.7.4
>
Oleksandr Tyshchenko Jan. 14, 2021, 3:31 p.m. UTC | #2
On 14.01.21 05:58, Wei Chen wrote:
> Hi Oleksandr,

Hi Wei


>
>> -----Original Message-----
>> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of
>> Oleksandr Tyshchenko
>> Sent: 2021年1月13日 5:52
>> To: xen-devel@lists.xenproject.org
>> Cc: Julien Grall <Julien.Grall@arm.com>; Jan Beulich <jbeulich@suse.com>;
>> Andrew Cooper <andrew.cooper3@citrix.com>; Roger Pau Monné
>> <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; George Dunlap
>> <george.dunlap@citrix.com>; Ian Jackson <iwj@xenproject.org>; Julien Grall
>> <julien@xen.org>; Stefano Stabellini <sstabellini@kernel.org>; Volodymyr
>> Babchuk <Volodymyr_Babchuk@epam.com>; Oleksandr Tyshchenko
>> <oleksandr_tyshchenko@epam.com>
>> Subject: [PATCH V4 11/24] xen/mm: Make x86's
>> XENMEM_resource_ioreq_server handling common
>>
>> From: Julien Grall <julien.grall@arm.com>
>>
>> As x86 implementation of XENMEM_resource_ioreq_server can be
>> re-used on Arm later on, this patch makes it common and removes
>> arch_acquire_resource as unneeded.
>>
>> Also re-order #include-s alphabetically.
>>
>> This support is going to be used on Arm to be able run device
>> emulator outside of Xen hypervisor.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>> [On Arm only]
>> Tested-by: Wei Chen <Wei.Chen@arm.com>
>>
>> ---
>> Please note, this is a split/cleanup/hardening of Julien's PoC:
>> "Add support for Guest IO forwarding to a device emulator"
>>
>> Changes RFC -> V1:
>>     - no changes
>>
>> Changes V1 -> V2:
>>     - update the author of a patch
>>
>> Changes V2 -> V3:
>>     - don't wrap #include <xen/ioreq.h>
>>     - limit the number of #ifdef-s
>>     - re-order #include-s alphabetically
>>
>> Changes V3 -> V4:
>>     - rebase
>>     - Add Jan's R-b
>> ---
>>   xen/arch/x86/mm.c        | 44 ---------------------------------
>>   xen/common/memory.c      | 63
>> +++++++++++++++++++++++++++++++++++++++---------
>>   xen/include/asm-arm/mm.h |  8 ------
>>   xen/include/asm-x86/mm.h |  4 ---
>>   4 files changed, 51 insertions(+), 68 deletions(-)
>>
>> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
>> index f6e128e..54ac398 100644
>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -4587,50 +4587,6 @@ static int handle_iomem_range(unsigned long s,
>> unsigned long e, void *p)
>>       return err || s > e ? err : _handle_iomem_range(s, e, p);
>>   }
>>
>> -int arch_acquire_resource(struct domain *d, unsigned int type,
>> -                          unsigned int id, unsigned long frame,
>> -                          unsigned int nr_frames, xen_pfn_t mfn_list[])
>> -{
>> -    int rc;
>> -
>> -    switch ( type )
>> -    {
>> -#ifdef CONFIG_HVM
>> -    case XENMEM_resource_ioreq_server:
>> -    {
>> -        ioservid_t ioservid = id;
>> -        unsigned int i;
>> -
>> -        rc = -EINVAL;
>> -        if ( !is_hvm_domain(d) )
>> -            break;
>> -
>> -        if ( id != (unsigned int)ioservid )
>> -            break;
>> -
>> -        rc = 0;
>> -        for ( i = 0; i < nr_frames; i++ )
>> -        {
>> -            mfn_t mfn;
>> -
>> -            rc = hvm_get_ioreq_server_frame(d, id, frame + i, &mfn);
>> -            if ( rc )
>> -                break;
>> -
>> -            mfn_list[i] = mfn_x(mfn);
>> -        }
>> -        break;
>> -    }
>> -#endif
>> -
>> -    default:
>> -        rc = -EOPNOTSUPP;
>> -        break;
>> -    }
>> -
>> -    return rc;
>> -}
>> -
>>   long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void)
>> arg)
>>   {
>>       int rc;
>> diff --git a/xen/common/memory.c b/xen/common/memory.c
>> index b21b6c4..7e560b5 100644
>> --- a/xen/common/memory.c
>> +++ b/xen/common/memory.c
>> @@ -8,22 +8,23 @@
>>    */
>>
>>   #include <xen/domain_page.h>
>> -#include <xen/types.h>
>> +#include <xen/errno.h>
>> +#include <xen/event.h>
>> +#include <xen/grant_table.h>
>> +#include <xen/guest_access.h>
>> +#include <xen/hypercall.h>
>> +#include <xen/iocap.h>
>> +#include <xen/ioreq.h>
>>   #include <xen/lib.h>
>> +#include <xen/mem_access.h>
>>   #include <xen/mm.h>
>> +#include <xen/numa.h>
>> +#include <xen/paging.h>
>>   #include <xen/param.h>
>>   #include <xen/perfc.h>
>>   #include <xen/sched.h>
>> -#include <xen/event.h>
>> -#include <xen/paging.h>
>> -#include <xen/iocap.h>
>> -#include <xen/guest_access.h>
>> -#include <xen/hypercall.h>
>> -#include <xen/errno.h>
>> -#include <xen/numa.h>
>> -#include <xen/mem_access.h>
>>   #include <xen/trace.h>
>> -#include <xen/grant_table.h>
>> +#include <xen/types.h>
>>   #include <asm/current.h>
>>   #include <asm/hardirq.h>
>>   #include <asm/p2m.h>
>> @@ -1090,6 +1091,40 @@ static int acquire_grant_table(struct domain *d,
>> unsigned int id,
>>       return 0;
>>   }
>>
>> +static int acquire_ioreq_server(struct domain *d,
>> +                                unsigned int id,
>> +                                unsigned long frame,
>> +                                unsigned int nr_frames,
>> +                                xen_pfn_t mfn_list[])
>> +{
>> +#ifdef CONFIG_IOREQ_SERVER
>> +    ioservid_t ioservid = id;
>> +    unsigned int i;
>> +    int rc;
>> +
>> +    if ( !is_hvm_domain(d) )
>> +        return -EINVAL;
>> +
>> +    if ( id != (unsigned int)ioservid )
>> +        return -EINVAL;
>> +
>> +    for ( i = 0; i < nr_frames; i++ )
>> +    {
>> +        mfn_t mfn;
>> +
>> +        rc = hvm_get_ioreq_server_frame(d, id, frame + i, &mfn);
>> +        if ( rc )
>> +            return rc;
>> +
>> +        mfn_list[i] = mfn_x(mfn);
>> +    }
>> +
>> +    return 0;
>> +#else
>> +    return -EOPNOTSUPP;
>> +#endif
>> +}
>> +
>>   static int acquire_resource(
>>       XEN_GUEST_HANDLE_PARAM(xen_mem_acquire_resource_t) arg)
>>   {
>> @@ -1148,9 +1183,13 @@ static int acquire_resource(
>>                                    mfn_list);
>>           break;
>>
>> +    case XENMEM_resource_ioreq_server:
>> +        rc = acquire_ioreq_server(d, xmar.id, xmar.frame, xmar.nr_frames,
>> +                                  mfn_list);
>> +        break;
>> +
>>       default:
>> -        rc = arch_acquire_resource(d, xmar.type, xmar.id, xmar.frame,
>> -                                   xmar.nr_frames, mfn_list);
>> +        rc = -EOPNOTSUPP;
>>           break;
>>       }
>>
>> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
>> index f8ba49b..0b7de31 100644
>> --- a/xen/include/asm-arm/mm.h
>> +++ b/xen/include/asm-arm/mm.h
>> @@ -358,14 +358,6 @@ static inline void put_page_and_type(struct page_info
>> *page)
>>
>>   void clear_and_clean_page(struct page_info *page);
>>
>> -static inline
>> -int arch_acquire_resource(struct domain *d, unsigned int type, unsigned int id,
>> -                          unsigned long frame, unsigned int nr_frames,
>> -                          xen_pfn_t mfn_list[])
>> -{
>> -    return -EOPNOTSUPP;
>> -}
>> -
>>   unsigned int arch_get_dma_bitsize(void);
>>
> This change could not be applied to the latest staging branch.

Yes, thank you noticing that.  The code around was changed a bit (patch 
series is based on 10-days old staging), I will update for the next version.


>
>>   #endif /*  __ARCH_ARM_MM__ */
>> diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h
>> index deeba75..859214e 100644
>> --- a/xen/include/asm-x86/mm.h
>> +++ b/xen/include/asm-x86/mm.h
>> @@ -639,8 +639,4 @@ static inline bool arch_mfn_in_directmap(unsigned long
>> mfn)
>>       return mfn <= (virt_to_mfn(eva - 1) + 1);
>>   }
>>
>> -int arch_acquire_resource(struct domain *d, unsigned int type,
>> -                          unsigned int id, unsigned long frame,
>> -                          unsigned int nr_frames, xen_pfn_t mfn_list[]);
>> -
>>   #endif /* __ASM_X86_MM_H__ */
>> --
>> 2.7.4
>>
Alex Bennée Jan. 15, 2021, 2:35 p.m. UTC | #3
Oleksandr <olekstysh@gmail.com> writes:

> On 14.01.21 05:58, Wei Chen wrote:
>> Hi Oleksandr,
>
> Hi Wei
<snip>
>>> @@ -1090,6 +1091,40 @@ static int acquire_grant_table(struct domain *d,
>>> unsigned int id,
>>>       return 0;
>>>   }
>>>
>>> +static int acquire_ioreq_server(struct domain *d,
>>> +                                unsigned int id,
>>> +                                unsigned long frame,
>>> +                                unsigned int nr_frames,
>>> +                                xen_pfn_t mfn_list[])
>>> +{
>>> +#ifdef CONFIG_IOREQ_SERVER
>>> +    ioservid_t ioservid = id;
>>> +    unsigned int i;
>>> +    int rc;
>>> +
>>> +    if ( !is_hvm_domain(d) )
>>> +        return -EINVAL;
>>> +
>>> +    if ( id != (unsigned int)ioservid )
>>> +        return -EINVAL;
>>> +
>>> +    for ( i = 0; i < nr_frames; i++ )
>>> +    {
>>> +        mfn_t mfn;
>>> +
>>> +        rc = hvm_get_ioreq_server_frame(d, id, frame + i, &mfn);
>>> +        if ( rc )
>>> +            return rc;
>>> +
>>> +        mfn_list[i] = mfn_x(mfn);
>>> +    }
>>> +
>>> +    return 0;
>>> +#else
>>> +    return -EOPNOTSUPP;
>>> +#endif
>>> +}
>>> +
<snip>
>>>
>> This change could not be applied to the latest staging branch.
>
> Yes, thank you noticing that.  The code around was changed a bit (patch 
> series is based on 10-days old staging), I will update for the next
> version.

I think the commit that introduced config ARCH_ACQUIRE_RESOURCE could
probably be reverted as it achieves pretty much the same thing as the
above code by moving the logic into the common code path.

The only real practical difference is a inline stub vs a general purpose
function with an IOREQ specific #ifdeferry.
<snip>
Paul Durrant Jan. 18, 2021, 9:38 a.m. UTC | #4
> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Oleksandr Tyshchenko
> Sent: 12 January 2021 21:52
> To: xen-devel@lists.xenproject.org
> Cc: Julien Grall <julien.grall@arm.com>; Jan Beulich <jbeulich@suse.com>; Andrew Cooper
> <andrew.cooper3@citrix.com>; Roger Pau Monné <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; George
> Dunlap <george.dunlap@citrix.com>; Ian Jackson <iwj@xenproject.org>; Julien Grall <julien@xen.org>;
> Stefano Stabellini <sstabellini@kernel.org>; Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Oleksandr
> Tyshchenko <oleksandr_tyshchenko@epam.com>
> Subject: [PATCH V4 11/24] xen/mm: Make x86's XENMEM_resource_ioreq_server handling common
> 
> From: Julien Grall <julien.grall@arm.com>
> 
> As x86 implementation of XENMEM_resource_ioreq_server can be
> re-used on Arm later on, this patch makes it common and removes
> arch_acquire_resource as unneeded.
> 
> Also re-order #include-s alphabetically.
> 
> This support is going to be used on Arm to be able run device
> emulator outside of Xen hypervisor.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> [On Arm only]
> Tested-by: Wei Chen <Wei.Chen@arm.com>
> 

Reviewed-by: Paul Durrant <paul@xen.org>
Oleksandr Tyshchenko Jan. 18, 2021, 5:42 p.m. UTC | #5
On 15.01.21 16:35, Alex Bennée wrote:

Hi Alex

> Oleksandr <olekstysh@gmail.com> writes:
>
>> On 14.01.21 05:58, Wei Chen wrote:
>>> Hi Oleksandr,
>> Hi Wei
> <snip>
>>>> @@ -1090,6 +1091,40 @@ static int acquire_grant_table(struct domain *d,
>>>> unsigned int id,
>>>>        return 0;
>>>>    }
>>>>
>>>> +static int acquire_ioreq_server(struct domain *d,
>>>> +                                unsigned int id,
>>>> +                                unsigned long frame,
>>>> +                                unsigned int nr_frames,
>>>> +                                xen_pfn_t mfn_list[])
>>>> +{
>>>> +#ifdef CONFIG_IOREQ_SERVER
>>>> +    ioservid_t ioservid = id;
>>>> +    unsigned int i;
>>>> +    int rc;
>>>> +
>>>> +    if ( !is_hvm_domain(d) )
>>>> +        return -EINVAL;
>>>> +
>>>> +    if ( id != (unsigned int)ioservid )
>>>> +        return -EINVAL;
>>>> +
>>>> +    for ( i = 0; i < nr_frames; i++ )
>>>> +    {
>>>> +        mfn_t mfn;
>>>> +
>>>> +        rc = hvm_get_ioreq_server_frame(d, id, frame + i, &mfn);
>>>> +        if ( rc )
>>>> +            return rc;
>>>> +
>>>> +        mfn_list[i] = mfn_x(mfn);
>>>> +    }
>>>> +
>>>> +    return 0;
>>>> +#else
>>>> +    return -EOPNOTSUPP;
>>>> +#endif
>>>> +}
>>>> +
> <snip>
>>> This change could not be applied to the latest staging branch.
>> Yes, thank you noticing that.  The code around was changed a bit (patch
>> series is based on 10-days old staging), I will update for the next
>> version.
> I think the commit that introduced config ARCH_ACQUIRE_RESOURCE could
> probably be reverted as it achieves pretty much the same thing as the
> above code by moving the logic into the common code path.
>
> The only real practical difference is a inline stub vs a general purpose
> function with an IOREQ specific #ifdeferry.
> <snip>
Hmm, thank you for noticing that.
So, yes, I should either add an extra patch for V5 to revert 
ARCH_ACQUIRE_RESOURCE before applying this one
or rebase it to the current codebase (and likely drop all collected R-bs 
because of an additional changes of removing ARCH_ACQUIRE_RESOURCE bits).
diff mbox series

Patch

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index f6e128e..54ac398 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4587,50 +4587,6 @@  static int handle_iomem_range(unsigned long s, unsigned long e, void *p)
     return err || s > e ? err : _handle_iomem_range(s, e, p);
 }
 
-int arch_acquire_resource(struct domain *d, unsigned int type,
-                          unsigned int id, unsigned long frame,
-                          unsigned int nr_frames, xen_pfn_t mfn_list[])
-{
-    int rc;
-
-    switch ( type )
-    {
-#ifdef CONFIG_HVM
-    case XENMEM_resource_ioreq_server:
-    {
-        ioservid_t ioservid = id;
-        unsigned int i;
-
-        rc = -EINVAL;
-        if ( !is_hvm_domain(d) )
-            break;
-
-        if ( id != (unsigned int)ioservid )
-            break;
-
-        rc = 0;
-        for ( i = 0; i < nr_frames; i++ )
-        {
-            mfn_t mfn;
-
-            rc = hvm_get_ioreq_server_frame(d, id, frame + i, &mfn);
-            if ( rc )
-                break;
-
-            mfn_list[i] = mfn_x(mfn);
-        }
-        break;
-    }
-#endif
-
-    default:
-        rc = -EOPNOTSUPP;
-        break;
-    }
-
-    return rc;
-}
-
 long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     int rc;
diff --git a/xen/common/memory.c b/xen/common/memory.c
index b21b6c4..7e560b5 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -8,22 +8,23 @@ 
  */
 
 #include <xen/domain_page.h>
-#include <xen/types.h>
+#include <xen/errno.h>
+#include <xen/event.h>
+#include <xen/grant_table.h>
+#include <xen/guest_access.h>
+#include <xen/hypercall.h>
+#include <xen/iocap.h>
+#include <xen/ioreq.h>
 #include <xen/lib.h>
+#include <xen/mem_access.h>
 #include <xen/mm.h>
+#include <xen/numa.h>
+#include <xen/paging.h>
 #include <xen/param.h>
 #include <xen/perfc.h>
 #include <xen/sched.h>
-#include <xen/event.h>
-#include <xen/paging.h>
-#include <xen/iocap.h>
-#include <xen/guest_access.h>
-#include <xen/hypercall.h>
-#include <xen/errno.h>
-#include <xen/numa.h>
-#include <xen/mem_access.h>
 #include <xen/trace.h>
-#include <xen/grant_table.h>
+#include <xen/types.h>
 #include <asm/current.h>
 #include <asm/hardirq.h>
 #include <asm/p2m.h>
@@ -1090,6 +1091,40 @@  static int acquire_grant_table(struct domain *d, unsigned int id,
     return 0;
 }
 
+static int acquire_ioreq_server(struct domain *d,
+                                unsigned int id,
+                                unsigned long frame,
+                                unsigned int nr_frames,
+                                xen_pfn_t mfn_list[])
+{
+#ifdef CONFIG_IOREQ_SERVER
+    ioservid_t ioservid = id;
+    unsigned int i;
+    int rc;
+
+    if ( !is_hvm_domain(d) )
+        return -EINVAL;
+
+    if ( id != (unsigned int)ioservid )
+        return -EINVAL;
+
+    for ( i = 0; i < nr_frames; i++ )
+    {
+        mfn_t mfn;
+
+        rc = hvm_get_ioreq_server_frame(d, id, frame + i, &mfn);
+        if ( rc )
+            return rc;
+
+        mfn_list[i] = mfn_x(mfn);
+    }
+
+    return 0;
+#else
+    return -EOPNOTSUPP;
+#endif
+}
+
 static int acquire_resource(
     XEN_GUEST_HANDLE_PARAM(xen_mem_acquire_resource_t) arg)
 {
@@ -1148,9 +1183,13 @@  static int acquire_resource(
                                  mfn_list);
         break;
 
+    case XENMEM_resource_ioreq_server:
+        rc = acquire_ioreq_server(d, xmar.id, xmar.frame, xmar.nr_frames,
+                                  mfn_list);
+        break;
+
     default:
-        rc = arch_acquire_resource(d, xmar.type, xmar.id, xmar.frame,
-                                   xmar.nr_frames, mfn_list);
+        rc = -EOPNOTSUPP;
         break;
     }
 
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index f8ba49b..0b7de31 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -358,14 +358,6 @@  static inline void put_page_and_type(struct page_info *page)
 
 void clear_and_clean_page(struct page_info *page);
 
-static inline
-int arch_acquire_resource(struct domain *d, unsigned int type, unsigned int id,
-                          unsigned long frame, unsigned int nr_frames,
-                          xen_pfn_t mfn_list[])
-{
-    return -EOPNOTSUPP;
-}
-
 unsigned int arch_get_dma_bitsize(void);
 
 #endif /*  __ARCH_ARM_MM__ */
diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h
index deeba75..859214e 100644
--- a/xen/include/asm-x86/mm.h
+++ b/xen/include/asm-x86/mm.h
@@ -639,8 +639,4 @@  static inline bool arch_mfn_in_directmap(unsigned long mfn)
     return mfn <= (virt_to_mfn(eva - 1) + 1);
 }
 
-int arch_acquire_resource(struct domain *d, unsigned int type,
-                          unsigned int id, unsigned long frame,
-                          unsigned int nr_frames, xen_pfn_t mfn_list[]);
-
 #endif /* __ASM_X86_MM_H__ */