[V2,2/4] arm/xen: Switch to use gnttab_setup_auto_xlat_frames() for DT

Commit Message

Oleksandr Tyshchenko Oct. 26, 2021, 4:05 p.m. UTC

From: Oleksandr Tyshchenko <[e-mail]>

Read the start address of the grant table space from DT
(region 0).

This patch mostly restores behaviour before commit 3cf4095d7446
("arm/xen: Use xen_xlate_map_ballooned_pages to setup grant table")
but trying not to break the ACPI support added after that commit.
So the patch touches DT part only and leaves the ACPI part with
xen_xlate_map_ballooned_pages().

This is a preparation for using Xen extended region feature
where unused regions of guest physical address space (provided
by the hypervisor) will be used to create grant/foreign/whatever
mappings instead of wasting real RAM pages from the domain memory
for establishing these mappings.

The immediate benefit of this change:
- Avoid superpage shattering in Xen P2M when establishing
  stage-2 mapping (GFN <-> MFN) for the grant table space
- Avoid wasting real RAM pages (reducing the amount of memory
  usuable) for mapping grant table space
- The grant table space is always mapped at the exact
  same place (region 0 is reserved for the grant table)

Signed-off-by: Oleksandr Tyshchenko <[e-mail]>
---
Changes RFC -> V2:
   - new patch
---
 arch/arm/xen/enlighten.c | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

Comments

Stefano Stabellini Oct. 28, 2021, 1:28 a.m. UTC | #1

On Tue, 26 Oct 2021, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <[e-mail]>
> 
> Read the start address of the grant table space from DT
> (region 0).
> 
> This patch mostly restores behaviour before commit 3cf4095d7446
> ("arm/xen: Use xen_xlate_map_ballooned_pages to setup grant table")
> but trying not to break the ACPI support added after that commit.
> So the patch touches DT part only and leaves the ACPI part with
> xen_xlate_map_ballooned_pages().
> 
> This is a preparation for using Xen extended region feature
> where unused regions of guest physical address space (provided
> by the hypervisor) will be used to create grant/foreign/whatever
> mappings instead of wasting real RAM pages from the domain memory
> for establishing these mappings.
> 
> The immediate benefit of this change:
> - Avoid superpage shattering in Xen P2M when establishing
>   stage-2 mapping (GFN <-> MFN) for the grant table space
> - Avoid wasting real RAM pages (reducing the amount of memory
>   usuable) for mapping grant table space
> - The grant table space is always mapped at the exact
>   same place (region 0 is reserved for the grant table)
> 
> Signed-off-by: Oleksandr Tyshchenko <[e-mail]>
> ---
> Changes RFC -> V2:
>    - new patch
> ---
>  arch/arm/xen/enlighten.c | 32 +++++++++++++++++++++++++-------
>  1 file changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 7f1c106b..dea46ec 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -59,6 +59,9 @@ unsigned long xen_released_pages;
>  struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] __initdata;
>  
>  static __read_mostly unsigned int xen_events_irq;
> +static phys_addr_t xen_grant_frames;

__read_mostly


> +#define GRANT_TABLE_INDEX   0
>  
>  uint32_t xen_start_flags;
>  EXPORT_SYMBOL(xen_start_flags);
> @@ -303,6 +306,7 @@ static void __init xen_acpi_guest_init(void)
>  static void __init xen_dt_guest_init(void)
>  {
>  	struct device_node *xen_node;
> +	struct resource res;
>  
>  	xen_node = of_find_compatible_node(NULL, NULL, "xen,xen");
>  	if (!xen_node) {
> @@ -310,6 +314,12 @@ static void __init xen_dt_guest_init(void)
>  		return;
>  	}
>  
> +	if (of_address_to_resource(xen_node, GRANT_TABLE_INDEX, &res)) {
> +		pr_err("Xen grant table region is not found\n");
> +		return;
> +	}
> +	xen_grant_frames = res.start;
> +
>  	xen_events_irq = irq_of_parse_and_map(xen_node, 0);
>  }
>  
> @@ -317,16 +327,20 @@ static int __init xen_guest_init(void)
>  {
>  	struct xen_add_to_physmap xatp;
>  	struct shared_info *shared_info_page = NULL;
> -	int cpu;
> +	int rc, cpu;
>  
>  	if (!xen_domain())
>  		return 0;
>  
>  	if (!acpi_disabled)
>  		xen_acpi_guest_init();
> -	else
> +	else {
>  		xen_dt_guest_init();
>  
> +		if (!xen_grant_frames)
> +			return -ENODEV;

maybe we can avoid this, see below


> +	}
> +
>  	if (!xen_events_irq) {
>  		pr_err("Xen event channel interrupt not found\n");
>  		return -ENODEV;
> @@ -370,12 +384,16 @@ static int __init xen_guest_init(void)
>  	for_each_possible_cpu(cpu)
>  		per_cpu(xen_vcpu_id, cpu) = cpu;
>  
> -	xen_auto_xlat_grant_frames.count = gnttab_max_grant_frames();
> -	if (xen_xlate_map_ballooned_pages(&xen_auto_xlat_grant_frames.pfn,
> -					  &xen_auto_xlat_grant_frames.vaddr,
> -					  xen_auto_xlat_grant_frames.count)) {
> +	if (!acpi_disabled) {

To make the code more resilient couldn't we do:

if (!acpi_disabled || !xen_grant_frames) {


> +		xen_auto_xlat_grant_frames.count = gnttab_max_grant_frames();
> +		rc = xen_xlate_map_ballooned_pages(&xen_auto_xlat_grant_frames.pfn,
> +										   &xen_auto_xlat_grant_frames.vaddr,
> +										   xen_auto_xlat_grant_frames.count);
> +	} else
> +		rc = gnttab_setup_auto_xlat_frames(xen_grant_frames);
> +	if (rc) {
>  		free_percpu(xen_vcpu_info);
> -		return -ENOMEM;
> +		return rc;
>  	}
>  	gnttab_init();

Oleksandr Tyshchenko Nov. 10, 2021, 10:14 p.m. UTC | #2

On 28.10.21 04:28, Stefano Stabellini wrote:

Hi Stefano

I am sorry for the late response.

> On Tue, 26 Oct 2021, Oleksandr Tyshchenko wrote:
>> From: Oleksandr Tyshchenko <[e-mail]>
>>
>> Read the start address of the grant table space from DT
>> (region 0).
>>
>> This patch mostly restores behaviour before commit 3cf4095d7446
>> ("arm/xen: Use xen_xlate_map_ballooned_pages to setup grant table")
>> but trying not to break the ACPI support added after that commit.
>> So the patch touches DT part only and leaves the ACPI part with
>> xen_xlate_map_ballooned_pages().
>>
>> This is a preparation for using Xen extended region feature
>> where unused regions of guest physical address space (provided
>> by the hypervisor) will be used to create grant/foreign/whatever
>> mappings instead of wasting real RAM pages from the domain memory
>> for establishing these mappings.
>>
>> The immediate benefit of this change:
>> - Avoid superpage shattering in Xen P2M when establishing
>>    stage-2 mapping (GFN <-> MFN) for the grant table space
>> - Avoid wasting real RAM pages (reducing the amount of memory
>>    usuable) for mapping grant table space
>> - The grant table space is always mapped at the exact
>>    same place (region 0 is reserved for the grant table)
>>
>> Signed-off-by: Oleksandr Tyshchenko <[e-mail]>
>> ---
>> Changes RFC -> V2:
>>     - new patch
>> ---
>>   arch/arm/xen/enlighten.c | 32 +++++++++++++++++++++++++-------
>>   1 file changed, 25 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
>> index 7f1c106b..dea46ec 100644
>> --- a/arch/arm/xen/enlighten.c
>> +++ b/arch/arm/xen/enlighten.c
>> @@ -59,6 +59,9 @@ unsigned long xen_released_pages;
>>   struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] __initdata;
>>   
>>   static __read_mostly unsigned int xen_events_irq;
>> +static phys_addr_t xen_grant_frames;
> __read_mostly

ok


>
>
>> +#define GRANT_TABLE_INDEX   0
>>   
>>   uint32_t xen_start_flags;
>>   EXPORT_SYMBOL(xen_start_flags);
>> @@ -303,6 +306,7 @@ static void __init xen_acpi_guest_init(void)
>>   static void __init xen_dt_guest_init(void)
>>   {
>>   	struct device_node *xen_node;
>> +	struct resource res;
>>   
>>   	xen_node = of_find_compatible_node(NULL, NULL, "xen,xen");
>>   	if (!xen_node) {
>> @@ -310,6 +314,12 @@ static void __init xen_dt_guest_init(void)
>>   		return;
>>   	}
>>   
>> +	if (of_address_to_resource(xen_node, GRANT_TABLE_INDEX, &res)) {
>> +		pr_err("Xen grant table region is not found\n");
>> +		return;
>> +	}
>> +	xen_grant_frames = res.start;
>> +
>>   	xen_events_irq = irq_of_parse_and_map(xen_node, 0);
>>   }
>>   
>> @@ -317,16 +327,20 @@ static int __init xen_guest_init(void)
>>   {
>>   	struct xen_add_to_physmap xatp;
>>   	struct shared_info *shared_info_page = NULL;
>> -	int cpu;
>> +	int rc, cpu;
>>   
>>   	if (!xen_domain())
>>   		return 0;
>>   
>>   	if (!acpi_disabled)
>>   		xen_acpi_guest_init();
>> -	else
>> +	else {
>>   		xen_dt_guest_init();
>>   
>> +		if (!xen_grant_frames)
>> +			return -ENODEV;
> maybe we can avoid this, see below
>
>
>> +	}
>> +
>>   	if (!xen_events_irq) {
>>   		pr_err("Xen event channel interrupt not found\n");
>>   		return -ENODEV;
>> @@ -370,12 +384,16 @@ static int __init xen_guest_init(void)
>>   	for_each_possible_cpu(cpu)
>>   		per_cpu(xen_vcpu_id, cpu) = cpu;
>>   
>> -	xen_auto_xlat_grant_frames.count = gnttab_max_grant_frames();
>> -	if (xen_xlate_map_ballooned_pages(&xen_auto_xlat_grant_frames.pfn,
>> -					  &xen_auto_xlat_grant_frames.vaddr,
>> -					  xen_auto_xlat_grant_frames.count)) {
>> +	if (!acpi_disabled) {
> To make the code more resilient couldn't we do:
>
> if (!acpi_disabled || !xen_grant_frames) {
I think, we can.

On the one hand, indeed the code more resilient and less change.
 From the other hand if grant table region is not found then something 
weird happened as region 0 is always present in reg property if 
hypervisor node is exposed to the guest.
The behavior before commit 3cf4095d7446 ("arm/xen: Use 
xen_xlate_map_ballooned_pages to setup grant table") was exactly the 
same in the context of the failure if region wasn't found.

...

Well, if we want to make code more resilient, I will update. But, looks 
like we also need to switch actions in xen_dt_guest_init() in order to 
process xen_events_irq before xen_grant_frames, otherwise we may return 
after failing with region and end up not initializing xen_events_irq so 
xen_guest_init() will fail earlier than reaches that check.
What do you think?


>
>> +		xen_auto_xlat_grant_frames.count = gnttab_max_grant_frames();
>> +		rc = xen_xlate_map_ballooned_pages(&xen_auto_xlat_grant_frames.pfn,
>> +										   &xen_auto_xlat_grant_frames.vaddr,
>> +										   xen_auto_xlat_grant_frames.count);
>> +	} else
>> +		rc = gnttab_setup_auto_xlat_frames(xen_grant_frames);
>> +	if (rc) {
>>   		free_percpu(xen_vcpu_info);
>> -		return -ENOMEM;
>> +		return rc;
>>   	}
>>   	gnttab_init();

Stefano Stabellini Nov. 19, 2021, 12:32 a.m. UTC | #3

On Thu, 11 Nov 2021, Oleksandr wrote:
> On 28.10.21 04:28, Stefano Stabellini wrote:
> 
> Hi Stefano
> 
> I am sorry for the late response.
> 
> > On Tue, 26 Oct 2021, Oleksandr Tyshchenko wrote:
> > > From: Oleksandr Tyshchenko <[e-mail]>
> > > 
> > > Read the start address of the grant table space from DT
> > > (region 0).
> > > 
> > > This patch mostly restores behaviour before commit 3cf4095d7446
> > > ("arm/xen: Use xen_xlate_map_ballooned_pages to setup grant table")
> > > but trying not to break the ACPI support added after that commit.
> > > So the patch touches DT part only and leaves the ACPI part with
> > > xen_xlate_map_ballooned_pages().
> > > 
> > > This is a preparation for using Xen extended region feature
> > > where unused regions of guest physical address space (provided
> > > by the hypervisor) will be used to create grant/foreign/whatever
> > > mappings instead of wasting real RAM pages from the domain memory
> > > for establishing these mappings.
> > > 
> > > The immediate benefit of this change:
> > > - Avoid superpage shattering in Xen P2M when establishing
> > >    stage-2 mapping (GFN <-> MFN) for the grant table space
> > > - Avoid wasting real RAM pages (reducing the amount of memory
> > >    usuable) for mapping grant table space
> > > - The grant table space is always mapped at the exact
> > >    same place (region 0 is reserved for the grant table)
> > > 
> > > Signed-off-by: Oleksandr Tyshchenko <[e-mail]>
> > > ---
> > > Changes RFC -> V2:
> > >     - new patch
> > > ---
> > >   arch/arm/xen/enlighten.c | 32 +++++++++++++++++++++++++-------
> > >   1 file changed, 25 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> > > index 7f1c106b..dea46ec 100644
> > > --- a/arch/arm/xen/enlighten.c
> > > +++ b/arch/arm/xen/enlighten.c
> > > @@ -59,6 +59,9 @@ unsigned long xen_released_pages;
> > >   struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS]
> > > __initdata;
> > >     static __read_mostly unsigned int xen_events_irq;
> > > +static phys_addr_t xen_grant_frames;
> > __read_mostly
> 
> ok
> 
> 
> > 
> > 
> > > +#define GRANT_TABLE_INDEX   0
> > >     uint32_t xen_start_flags;
> > >   EXPORT_SYMBOL(xen_start_flags);
> > > @@ -303,6 +306,7 @@ static void __init xen_acpi_guest_init(void)
> > >   static void __init xen_dt_guest_init(void)
> > >   {
> > >   	struct device_node *xen_node;
> > > +	struct resource res;
> > >     	xen_node = of_find_compatible_node(NULL, NULL, "xen,xen");
> > >   	if (!xen_node) {
> > > @@ -310,6 +314,12 @@ static void __init xen_dt_guest_init(void)
> > >   		return;
> > >   	}
> > >   +	if (of_address_to_resource(xen_node, GRANT_TABLE_INDEX, &res)) {
> > > +		pr_err("Xen grant table region is not found\n");
> > > +		return;
> > > +	}
> > > +	xen_grant_frames = res.start;
> > > +
> > >   	xen_events_irq = irq_of_parse_and_map(xen_node, 0);
> > >   }
> > >   @@ -317,16 +327,20 @@ static int __init xen_guest_init(void)
> > >   {
> > >   	struct xen_add_to_physmap xatp;
> > >   	struct shared_info *shared_info_page = NULL;
> > > -	int cpu;
> > > +	int rc, cpu;
> > >     	if (!xen_domain())
> > >   		return 0;
> > >     	if (!acpi_disabled)
> > >   		xen_acpi_guest_init();
> > > -	else
> > > +	else {
> > >   		xen_dt_guest_init();
> > >   +		if (!xen_grant_frames)
> > > +			return -ENODEV;
> > maybe we can avoid this, see below
> > 
> > 
> > > +	}
> > > +
> > >   	if (!xen_events_irq) {
> > >   		pr_err("Xen event channel interrupt not found\n");
> > >   		return -ENODEV;
> > > @@ -370,12 +384,16 @@ static int __init xen_guest_init(void)
> > >   	for_each_possible_cpu(cpu)
> > >   		per_cpu(xen_vcpu_id, cpu) = cpu;
> > >   -	xen_auto_xlat_grant_frames.count = gnttab_max_grant_frames();
> > > -	if (xen_xlate_map_ballooned_pages(&xen_auto_xlat_grant_frames.pfn,
> > > -					  &xen_auto_xlat_grant_frames.vaddr,
> > > -					  xen_auto_xlat_grant_frames.count)) {
> > > +	if (!acpi_disabled) {
> > To make the code more resilient couldn't we do:
> > 
> > if (!acpi_disabled || !xen_grant_frames) {
> I think, we can.
> 
> On the one hand, indeed the code more resilient and less change.
> From the other hand if grant table region is not found then something weird
> happened as region 0 is always present in reg property if hypervisor node is
> exposed to the guest.
> The behavior before commit 3cf4095d7446 ("arm/xen: Use
> xen_xlate_map_ballooned_pages to setup grant table") was exactly the same in
> the context of the failure if region wasn't found.
> 
> ...
> 
> Well, if we want to make code more resilient, I will update. But, looks like
> we also need to switch actions in xen_dt_guest_init() in order to process
> xen_events_irq before xen_grant_frames, otherwise we may return after failing
> with region and end up not initializing xen_events_irq so xen_guest_init()
> will fail earlier than reaches that check.
> What do you think?
 
Yes, you are right. I was re-reading the patch to refresh my memory and
I noticed immediately that xen_dt_guest_init also need to be changed so
that xen_events_irq is set before xen_grant_frames.
 
I think it is a minor change that doesn't add complexity but make the
code more robust so I think it is a good idea

 
> > > +		xen_auto_xlat_grant_frames.count = gnttab_max_grant_frames();
> > > +		rc =
> > > xen_xlate_map_ballooned_pages(&xen_auto_xlat_grant_frames.pfn,
> > > +
> > > &xen_auto_xlat_grant_frames.vaddr,
> > > +
> > > xen_auto_xlat_grant_frames.count);
> > > +	} else
> > > +		rc = gnttab_setup_auto_xlat_frames(xen_grant_frames);
> > > +	if (rc) {
> > >   		free_percpu(xen_vcpu_info);
> > > -		return -ENOMEM;
> > > +		return rc;
> > >   	}
> > >   	gnttab_init();

Oleksandr Tyshchenko Nov. 19, 2021, 6:25 p.m. UTC | #4

On 19.11.21 02:32, Stefano Stabellini wrote:

Hi Stefano

> On Thu, 11 Nov 2021, Oleksandr wrote:
>> On 28.10.21 04:28, Stefano Stabellini wrote:
>>
>> Hi Stefano
>>
>> I am sorry for the late response.
>>
>>> On Tue, 26 Oct 2021, Oleksandr Tyshchenko wrote:
>>>> From: Oleksandr Tyshchenko <[e-mail]>
>>>>
>>>> Read the start address of the grant table space from DT
>>>> (region 0).
>>>>
>>>> This patch mostly restores behaviour before commit 3cf4095d7446
>>>> ("arm/xen: Use xen_xlate_map_ballooned_pages to setup grant table")
>>>> but trying not to break the ACPI support added after that commit.
>>>> So the patch touches DT part only and leaves the ACPI part with
>>>> xen_xlate_map_ballooned_pages().
>>>>
>>>> This is a preparation for using Xen extended region feature
>>>> where unused regions of guest physical address space (provided
>>>> by the hypervisor) will be used to create grant/foreign/whatever
>>>> mappings instead of wasting real RAM pages from the domain memory
>>>> for establishing these mappings.
>>>>
>>>> The immediate benefit of this change:
>>>> - Avoid superpage shattering in Xen P2M when establishing
>>>>     stage-2 mapping (GFN <-> MFN) for the grant table space
>>>> - Avoid wasting real RAM pages (reducing the amount of memory
>>>>     usuable) for mapping grant table space
>>>> - The grant table space is always mapped at the exact
>>>>     same place (region 0 is reserved for the grant table)
>>>>
>>>> Signed-off-by: Oleksandr Tyshchenko <[e-mail]>
>>>> ---
>>>> Changes RFC -> V2:
>>>>      - new patch
>>>> ---
>>>>    arch/arm/xen/enlighten.c | 32 +++++++++++++++++++++++++-------
>>>>    1 file changed, 25 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
>>>> index 7f1c106b..dea46ec 100644
>>>> --- a/arch/arm/xen/enlighten.c
>>>> +++ b/arch/arm/xen/enlighten.c
>>>> @@ -59,6 +59,9 @@ unsigned long xen_released_pages;
>>>>    struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS]
>>>> __initdata;
>>>>      static __read_mostly unsigned int xen_events_irq;
>>>> +static phys_addr_t xen_grant_frames;
>>> __read_mostly
>> ok
>>
>>
>>>
>>>> +#define GRANT_TABLE_INDEX   0
>>>>      uint32_t xen_start_flags;
>>>>    EXPORT_SYMBOL(xen_start_flags);
>>>> @@ -303,6 +306,7 @@ static void __init xen_acpi_guest_init(void)
>>>>    static void __init xen_dt_guest_init(void)
>>>>    {
>>>>    	struct device_node *xen_node;
>>>> +	struct resource res;
>>>>      	xen_node = of_find_compatible_node(NULL, NULL, "xen,xen");
>>>>    	if (!xen_node) {
>>>> @@ -310,6 +314,12 @@ static void __init xen_dt_guest_init(void)
>>>>    		return;
>>>>    	}
>>>>    +	if (of_address_to_resource(xen_node, GRANT_TABLE_INDEX, &res)) {
>>>> +		pr_err("Xen grant table region is not found\n");
>>>> +		return;
>>>> +	}
>>>> +	xen_grant_frames = res.start;
>>>> +
>>>>    	xen_events_irq = irq_of_parse_and_map(xen_node, 0);
>>>>    }
>>>>    @@ -317,16 +327,20 @@ static int __init xen_guest_init(void)
>>>>    {
>>>>    	struct xen_add_to_physmap xatp;
>>>>    	struct shared_info *shared_info_page = NULL;
>>>> -	int cpu;
>>>> +	int rc, cpu;
>>>>      	if (!xen_domain())
>>>>    		return 0;
>>>>      	if (!acpi_disabled)
>>>>    		xen_acpi_guest_init();
>>>> -	else
>>>> +	else {
>>>>    		xen_dt_guest_init();
>>>>    +		if (!xen_grant_frames)
>>>> +			return -ENODEV;
>>> maybe we can avoid this, see below
>>>
>>>
>>>> +	}
>>>> +
>>>>    	if (!xen_events_irq) {
>>>>    		pr_err("Xen event channel interrupt not found\n");
>>>>    		return -ENODEV;
>>>> @@ -370,12 +384,16 @@ static int __init xen_guest_init(void)
>>>>    	for_each_possible_cpu(cpu)
>>>>    		per_cpu(xen_vcpu_id, cpu) = cpu;
>>>>    -	xen_auto_xlat_grant_frames.count = gnttab_max_grant_frames();
>>>> -	if (xen_xlate_map_ballooned_pages(&xen_auto_xlat_grant_frames.pfn,
>>>> -					  &xen_auto_xlat_grant_frames.vaddr,
>>>> -					  xen_auto_xlat_grant_frames.count)) {
>>>> +	if (!acpi_disabled) {
>>> To make the code more resilient couldn't we do:
>>>
>>> if (!acpi_disabled || !xen_grant_frames) {
>> I think, we can.
>>
>> On the one hand, indeed the code more resilient and less change.
>>  From the other hand if grant table region is not found then something weird
>> happened as region 0 is always present in reg property if hypervisor node is
>> exposed to the guest.
>> The behavior before commit 3cf4095d7446 ("arm/xen: Use
>> xen_xlate_map_ballooned_pages to setup grant table") was exactly the same in
>> the context of the failure if region wasn't found.
>>
>> ...
>>
>> Well, if we want to make code more resilient, I will update. But, looks like
>> we also need to switch actions in xen_dt_guest_init() in order to process
>> xen_events_irq before xen_grant_frames, otherwise we may return after failing
>> with region and end up not initializing xen_events_irq so xen_guest_init()
>> will fail earlier than reaches that check.
>> What do you think?
>   
> Yes, you are right. I was re-reading the patch to refresh my memory and
> I noticed immediately that xen_dt_guest_init also need to be changed so
> that xen_events_irq is set before xen_grant_frames.
>   
> I think it is a minor change that doesn't add complexity but make the
> code more robust so I think it is a good idea


Great, thank you. Will do in next version.


>
>   
>>>> +		xen_auto_xlat_grant_frames.count = gnttab_max_grant_frames();
>>>> +		rc =
>>>> xen_xlate_map_ballooned_pages(&xen_auto_xlat_grant_frames.pfn,
>>>> +
>>>> &xen_auto_xlat_grant_frames.vaddr,
>>>> +
>>>> xen_auto_xlat_grant_frames.count);
>>>> +	} else
>>>> +		rc = gnttab_setup_auto_xlat_frames(xen_grant_frames);
>>>> +	if (rc) {
>>>>    		free_percpu(xen_vcpu_info);
>>>> -		return -ENOMEM;
>>>> +		return rc;
>>>>    	}
>>>>    	gnttab_init();

Patch

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 7f1c106b..dea46ec 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -59,6 +59,9 @@  unsigned long xen_released_pages;
 struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] __initdata;
 
 static __read_mostly unsigned int xen_events_irq;
+static phys_addr_t xen_grant_frames;
+
+#define GRANT_TABLE_INDEX   0
 
 uint32_t xen_start_flags;
 EXPORT_SYMBOL(xen_start_flags);
@@ -303,6 +306,7 @@  static void __init xen_acpi_guest_init(void)
 static void __init xen_dt_guest_init(void)
 {
 	struct device_node *xen_node;
+	struct resource res;
 
 	xen_node = of_find_compatible_node(NULL, NULL, "xen,xen");
 	if (!xen_node) {
@@ -310,6 +314,12 @@  static void __init xen_dt_guest_init(void)
 		return;
 	}
 
+	if (of_address_to_resource(xen_node, GRANT_TABLE_INDEX, &res)) {
+		pr_err("Xen grant table region is not found\n");
+		return;
+	}
+	xen_grant_frames = res.start;
+
 	xen_events_irq = irq_of_parse_and_map(xen_node, 0);
 }
 
@@ -317,16 +327,20 @@  static int __init xen_guest_init(void)
 {
 	struct xen_add_to_physmap xatp;
 	struct shared_info *shared_info_page = NULL;
-	int cpu;
+	int rc, cpu;
 
 	if (!xen_domain())
 		return 0;
 
 	if (!acpi_disabled)
 		xen_acpi_guest_init();
-	else
+	else {
 		xen_dt_guest_init();
 
+		if (!xen_grant_frames)
+			return -ENODEV;
+	}
+
 	if (!xen_events_irq) {
 		pr_err("Xen event channel interrupt not found\n");
 		return -ENODEV;
@@ -370,12 +384,16 @@  static int __init xen_guest_init(void)
 	for_each_possible_cpu(cpu)
 		per_cpu(xen_vcpu_id, cpu) = cpu;
 
-	xen_auto_xlat_grant_frames.count = gnttab_max_grant_frames();
-	if (xen_xlate_map_ballooned_pages(&xen_auto_xlat_grant_frames.pfn,
-					  &xen_auto_xlat_grant_frames.vaddr,
-					  xen_auto_xlat_grant_frames.count)) {
+	if (!acpi_disabled) {
+		xen_auto_xlat_grant_frames.count = gnttab_max_grant_frames();
+		rc = xen_xlate_map_ballooned_pages(&xen_auto_xlat_grant_frames.pfn,
+										   &xen_auto_xlat_grant_frames.vaddr,
+										   xen_auto_xlat_grant_frames.count);
+	} else
+		rc = gnttab_setup_auto_xlat_frames(xen_grant_frames);
+	if (rc) {
 		free_percpu(xen_vcpu_info);
-		return -ENOMEM;
+		return rc;
 	}
 	gnttab_init();