Re: [Xen-devel] [PATCH v2 1/2] xen/arm: alternative: Clean-up __apply_alternatives

[Julien Grall]

Hi Konrad,

On 07/09/2016 15:13, Konrad Rzeszutek Wilk wrote:

On Wed, Sep 07, 2016 at 01:50:43PM +0100, Julien Grall wrote:

diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c
index 8ee5a11..0ca97b9 100644
--- a/xen/arch/arm/alternative.c
+++ b/xen/arch/arm/alternative.c
@@ -99,21 +99,21 @@ static int __apply_alternatives(const struct alt_region 
*region)
 const struct alt_instr *alt;
 const u32 *origptr, *replptr;
 u32 *writeptr, *writemap;
-mfn_t text_mfn = _mfn(virt_to_mfn(_stext));
-unsigned int text_order = get_order_from_bytes(_end - _start);
+mfn_t xen_mfn = _mfn(virt_to_mfn(_start));
+unsigned int xen_order = get_order_from_bytes(_end - _start);

 printk(XENLOG_INFO "alternatives: Patching kernel code\n");

 /*
- * The text section is read-only. So re-map Xen to be able to patch
- * the code.
+ * The text and inittext section are read-only. So re-map Xen to be
+ * able to patch the code.
  */
-writemap = __vmap(_mfn, 1 << text_order, 1, 1, PAGE_HYPERVISOR,
+writemap = __vmap(_mfn, 1 << xen_order, 1, 1, PAGE_HYPERVISOR,


Do you want to make it 1U? (You pointed that out in my patchset so perhaps you
want it here?)


Good point, I used 1U whilst moving the code in patch #2 but forgot here.

I will update the patch and resend it.

Regards,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 1/2] xen/arm: alternative: Clean-up __apply_alternatives

[Konrad Rzeszutek Wilk]

On Wed, Sep 07, 2016 at 01:50:43PM +0100, Julien Grall wrote:
> This patch contains only renaming and comment update. There are no
> functional changes:
> - Don't mix _start and _stext, they both point to the same address
> but the former makes more sense (we are mapping the Xen binary, not
> only the text section).
> - s/text_mfn/xen_mfn/ and s/text_order/xen_order/ to make clear that
> we map the Xen binary.
> - Mention about inittext as alternative may patch this section.
> 
> Signed-off-by: Konrad Rzeszutek Wilk 
> Signed-off-by: Julien Grall 
> 
> ---
> Konrad, I added your signed-off by because I squashed your patch [1]
> in it. Let me know if there is any issue for that.

No issues. Albeit it may be odd for me to review my own patch :-)

See below

> 
> [1] https://lists.xen.org/archives/html/xen-devel/2016-08/msg02890.html
> 
> Changes in v2:
> - Patch added
> ---
>  xen/arch/arm/alternative.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c
> index 8ee5a11..0ca97b9 100644
> --- a/xen/arch/arm/alternative.c
> +++ b/xen/arch/arm/alternative.c
> @@ -99,21 +99,21 @@ static int __apply_alternatives(const struct alt_region 
> *region)
>  const struct alt_instr *alt;
>  const u32 *origptr, *replptr;
>  u32 *writeptr, *writemap;
> -mfn_t text_mfn = _mfn(virt_to_mfn(_stext));
> -unsigned int text_order = get_order_from_bytes(_end - _start);
> +mfn_t xen_mfn = _mfn(virt_to_mfn(_start));
> +unsigned int xen_order = get_order_from_bytes(_end - _start);
>  
>  printk(XENLOG_INFO "alternatives: Patching kernel code\n");
>  
>  /*
> - * The text section is read-only. So re-map Xen to be able to patch
> - * the code.
> + * The text and inittext section are read-only. So re-map Xen to be
> + * able to patch the code.
>   */
> -writemap = __vmap(_mfn, 1 << text_order, 1, 1, PAGE_HYPERVISOR,
> +writemap = __vmap(_mfn, 1 << xen_order, 1, 1, PAGE_HYPERVISOR,

Do you want to make it 1U? (You pointed that out in my patchset so perhaps you
want it here?)

>VMAP_DEFAULT);
>  if ( !writemap )
>  {
>  printk(XENLOG_ERR "alternatives: Unable to map the text section 
> (size %u)\n",
> -   1 << text_order);
> +   1 << xen_order);
>  return -ENOMEM;
>  }
>  
> -- 
> 1.9.1
> 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 1/2] xen/arm: alternative: Clean-up __apply_alternatives

[Julien Grall]

This patch contains only renaming and comment update. There are no
functional changes:
- Don't mix _start and _stext, they both point to the same address
but the former makes more sense (we are mapping the Xen binary, not
only the text section).
- s/text_mfn/xen_mfn/ and s/text_order/xen_order/ to make clear that
we map the Xen binary.
- Mention about inittext as alternative may patch this section.

Signed-off-by: Konrad Rzeszutek Wilk 
Signed-off-by: Julien Grall 

---
Konrad, I added your signed-off by because I squashed your patch [1]
in it. Let me know if there is any issue for that.

[1] https://lists.xen.org/archives/html/xen-devel/2016-08/msg02890.html

Changes in v2:
- Patch added
---
 xen/arch/arm/alternative.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c
index 8ee5a11..0ca97b9 100644
--- a/xen/arch/arm/alternative.c
+++ b/xen/arch/arm/alternative.c
@@ -99,21 +99,21 @@ static int __apply_alternatives(const struct alt_region 
*region)
 const struct alt_instr *alt;
 const u32 *origptr, *replptr;
 u32 *writeptr, *writemap;
-mfn_t text_mfn = _mfn(virt_to_mfn(_stext));
-unsigned int text_order = get_order_from_bytes(_end - _start);
+mfn_t xen_mfn = _mfn(virt_to_mfn(_start));
+unsigned int xen_order = get_order_from_bytes(_end - _start);
 
 printk(XENLOG_INFO "alternatives: Patching kernel code\n");
 
 /*
- * The text section is read-only. So re-map Xen to be able to patch
- * the code.
+ * The text and inittext section are read-only. So re-map Xen to be
+ * able to patch the code.
  */
-writemap = __vmap(_mfn, 1 << text_order, 1, 1, PAGE_HYPERVISOR,
+writemap = __vmap(_mfn, 1 << xen_order, 1, 1, PAGE_HYPERVISOR,
   VMAP_DEFAULT);
 if ( !writemap )
 {
 printk(XENLOG_ERR "alternatives: Unable to map the text section (size 
%u)\n",
-   1 << text_order);
+   1 << xen_order);
 return -ENOMEM;
 }
 
-- 
1.9.1


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel