[Xen-devel] [PATCH for-4.13] clang: do not enable live-patching support

[Xen-devel] [PATCH for-4.13] clang: do not enable live-patching support

[Roger Pau Monne]

Live-patching requires unique symbols, and sadly the clang build
generates a lot of duplicate symbols:

Duplicate symbol 'asid.c#get_cpu_info' (ffff82d0803032c0 != ffff82d0802e0f50)
Duplicate symbol 'asid.c#get_cpu_info_from_stack' (ffff82d0802e1080 != ffff82d0803032f0)
Duplicate symbol 'ats.c#__list_add' (ffff82d080260a00 != ffff82d080267c70)
Duplicate symbol 'boot.c#constant_test_bit' (ffff82d08040ea60 != ffff82d0804372f0)
Duplicate symbol 'common.c#clear_bit' (ffff82d080332440 != ffff82d0802d33b0)
Duplicate symbol 'common.c#constant_test_bit' (ffff82d080332340 != ffff82d0802d2220)
Duplicate symbol 'common.c#cpumask_check' (ffff82d0802d3370 != ffff82d080337b60)
Duplicate symbol 'common.c#get_cpu_info' (ffff82d0802d22b0 != ffff82d080331590)
Duplicate symbol 'common.c#get_cpu_info_from_stack' (ffff82d0802d31c0 != ffff82d0803374b0)
Duplicate symbol 'common.c#pfn_to_pdx' (ffff82d0802d3270 != ffff82d080331e00)
Duplicate symbol 'common.c#test_and_set_bit' (ffff82d0802d3360 != ffff82d080332250)
Duplicate symbol 'common.c#variable_clear_bit' (ffff82d0802d2270 != ffff82d080337b50)
Duplicate symbol 'compat.c#get_cpu_info' (ffff82d08026eab0 != ffff82d080200460)
Duplicate symbol 'compat.c#get_cpu_info_from_stack' (ffff82d08026ebd0 != ffff82d080200f70)
Duplicate symbol 'cpu_idle.c#get_cpu_info' (ffff82d0802ccb00 != ffff82d08035fcc0)
[...]

For the time being disable live-patching when building with clang,
since duplicate symbols will trigger a build failure because
ENFORCE_UNIQUE_SYMBOLS is now also enabled by default in conjunction
with live-patching.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Jürgen Groß <jgross@suse.com>
---
 Config.mk          | 2 ++
 xen/common/Kconfig | 6 +++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/Config.mk b/Config.mk
index d8f90d75b3..009abda225 100644
--- a/Config.mk
+++ b/Config.mk
@@ -157,6 +157,8 @@ ifndef XEN_HAS_CHECKPOLICY
     export XEN_HAS_CHECKPOLICY
 endif
 
+export XEN_BUILD_WITH_CLANG = $(clang)
+
 # as-insn: Check whether assembler supports an instruction.
 # Usage: cflags-y += $(call as-insn,CC FLAGS,"insn",option-yes,option-no)
 as-insn = $(if $(shell echo 'void _(void) { asm volatile ( $(2) ); }' \
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index f754741972..097996fc6c 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -80,6 +80,10 @@ config HAS_CHECKPOLICY
 	string
 	option env="XEN_HAS_CHECKPOLICY"
 
+config BUILD_WITH_CLANG
+	string
+	option env="XEN_BUILD_WITH_CLANG"
+
 menu "Speculative hardening"
 
 config SPECULATIVE_HARDEN_ARRAY
@@ -350,7 +354,7 @@ config CRYPTO
 config LIVEPATCH
 	bool "Live patching support"
 	default X86
-	depends on HAS_BUILD_ID = "y"
+	depends on HAS_BUILD_ID = "y" && BUILD_WITH_CLANG != "y"
 	---help---
 	  Allows a running Xen hypervisor to be dynamically patched using
 	  binary patches without rebooting. This is primarily used to binarily
-- 
2.24.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH for-4.13] clang: do not enable live-patching support

[Jan Beulich]

On 27.11.2019 17:01, Roger Pau Monne wrote:
> Live-patching requires unique symbols, and sadly the clang build
> generates a lot of duplicate symbols:
> 
> Duplicate symbol 'asid.c#get_cpu_info' (ffff82d0803032c0 != ffff82d0802e0f50)
> Duplicate symbol 'asid.c#get_cpu_info_from_stack' (ffff82d0802e1080 != ffff82d0803032f0)
> Duplicate symbol 'ats.c#__list_add' (ffff82d080260a00 != ffff82d080267c70)
> Duplicate symbol 'boot.c#constant_test_bit' (ffff82d08040ea60 != ffff82d0804372f0)
> Duplicate symbol 'common.c#clear_bit' (ffff82d080332440 != ffff82d0802d33b0)
> Duplicate symbol 'common.c#constant_test_bit' (ffff82d080332340 != ffff82d0802d2220)
> Duplicate symbol 'common.c#cpumask_check' (ffff82d0802d3370 != ffff82d080337b60)
> Duplicate symbol 'common.c#get_cpu_info' (ffff82d0802d22b0 != ffff82d080331590)
> Duplicate symbol 'common.c#get_cpu_info_from_stack' (ffff82d0802d31c0 != ffff82d0803374b0)
> Duplicate symbol 'common.c#pfn_to_pdx' (ffff82d0802d3270 != ffff82d080331e00)
> Duplicate symbol 'common.c#test_and_set_bit' (ffff82d0802d3360 != ffff82d080332250)
> Duplicate symbol 'common.c#variable_clear_bit' (ffff82d0802d2270 != ffff82d080337b50)
> Duplicate symbol 'compat.c#get_cpu_info' (ffff82d08026eab0 != ffff82d080200460)
> Duplicate symbol 'compat.c#get_cpu_info_from_stack' (ffff82d08026ebd0 != ffff82d080200f70)
> Duplicate symbol 'cpu_idle.c#get_cpu_info' (ffff82d0802ccb00 != ffff82d08035fcc0)
> [...]
> 
> For the time being disable live-patching when building with clang,
> since duplicate symbols will trigger a build failure because
> ENFORCE_UNIQUE_SYMBOLS is now also enabled by default in conjunction
> with live-patching.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

To be honest, as indicated before I'm inclined to nak this patch
on the basis that a proper solution has been posted almost 3 weeks
ago (and this was already v2). Nevertheless a remark here:

> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -80,6 +80,10 @@ config HAS_CHECKPOLICY
>  	string
>  	option env="XEN_HAS_CHECKPOLICY"
>  
> +config BUILD_WITH_CLANG
> +	string
> +	option env="XEN_BUILD_WITH_CLANG"

Instead of introducing a new option here, ...

> @@ -350,7 +354,7 @@ config CRYPTO
>  config LIVEPATCH
>  	bool "Live patching support"
>  	default X86
> -	depends on HAS_BUILD_ID = "y"
> +	depends on HAS_BUILD_ID = "y" && BUILD_WITH_CLANG != "y"

... seeing this, why don't you simply suppress HAS_BUILD_ID acquiring
a value of y in ./Config.mk (accompanied by a suitable comment)?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH for-4.13] clang: do not enable live-patching support

[George Dunlap]

On 11/27/19 4:14 PM, Jan Beulich wrote:
> On 27.11.2019 17:01, Roger Pau Monne wrote:
>> Live-patching requires unique symbols, and sadly the clang build
>> generates a lot of duplicate symbols:
>>
>> Duplicate symbol 'asid.c#get_cpu_info' (ffff82d0803032c0 != ffff82d0802e0f50)
>> Duplicate symbol 'asid.c#get_cpu_info_from_stack' (ffff82d0802e1080 != ffff82d0803032f0)
>> Duplicate symbol 'ats.c#__list_add' (ffff82d080260a00 != ffff82d080267c70)
>> Duplicate symbol 'boot.c#constant_test_bit' (ffff82d08040ea60 != ffff82d0804372f0)
>> Duplicate symbol 'common.c#clear_bit' (ffff82d080332440 != ffff82d0802d33b0)
>> Duplicate symbol 'common.c#constant_test_bit' (ffff82d080332340 != ffff82d0802d2220)
>> Duplicate symbol 'common.c#cpumask_check' (ffff82d0802d3370 != ffff82d080337b60)
>> Duplicate symbol 'common.c#get_cpu_info' (ffff82d0802d22b0 != ffff82d080331590)
>> Duplicate symbol 'common.c#get_cpu_info_from_stack' (ffff82d0802d31c0 != ffff82d0803374b0)
>> Duplicate symbol 'common.c#pfn_to_pdx' (ffff82d0802d3270 != ffff82d080331e00)
>> Duplicate symbol 'common.c#test_and_set_bit' (ffff82d0802d3360 != ffff82d080332250)
>> Duplicate symbol 'common.c#variable_clear_bit' (ffff82d0802d2270 != ffff82d080337b50)
>> Duplicate symbol 'compat.c#get_cpu_info' (ffff82d08026eab0 != ffff82d080200460)
>> Duplicate symbol 'compat.c#get_cpu_info_from_stack' (ffff82d08026ebd0 != ffff82d080200f70)
>> Duplicate symbol 'cpu_idle.c#get_cpu_info' (ffff82d0802ccb00 != ffff82d08035fcc0)
>> [...]
>>
>> For the time being disable live-patching when building with clang,
>> since duplicate symbols will trigger a build failure because
>> ENFORCE_UNIQUE_SYMBOLS is now also enabled by default in conjunction
>> with live-patching.
>>
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> To be honest, as indicated before I'm inclined to nak this patch
> on the basis that a proper solution has been posted almost 3 weeks
> ago (and this was already v2).

What's that patch waiting on?

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH for-4.13] clang: do not enable live-patching support

[Jan Beulich]

On 27.11.2019 17:21, George Dunlap wrote:
> On 11/27/19 4:14 PM, Jan Beulich wrote:
>> On 27.11.2019 17:01, Roger Pau Monne wrote:
>>> Live-patching requires unique symbols, and sadly the clang build
>>> generates a lot of duplicate symbols:
>>>
>>> Duplicate symbol 'asid.c#get_cpu_info' (ffff82d0803032c0 != ffff82d0802e0f50)
>>> Duplicate symbol 'asid.c#get_cpu_info_from_stack' (ffff82d0802e1080 != ffff82d0803032f0)
>>> Duplicate symbol 'ats.c#__list_add' (ffff82d080260a00 != ffff82d080267c70)
>>> Duplicate symbol 'boot.c#constant_test_bit' (ffff82d08040ea60 != ffff82d0804372f0)
>>> Duplicate symbol 'common.c#clear_bit' (ffff82d080332440 != ffff82d0802d33b0)
>>> Duplicate symbol 'common.c#constant_test_bit' (ffff82d080332340 != ffff82d0802d2220)
>>> Duplicate symbol 'common.c#cpumask_check' (ffff82d0802d3370 != ffff82d080337b60)
>>> Duplicate symbol 'common.c#get_cpu_info' (ffff82d0802d22b0 != ffff82d080331590)
>>> Duplicate symbol 'common.c#get_cpu_info_from_stack' (ffff82d0802d31c0 != ffff82d0803374b0)
>>> Duplicate symbol 'common.c#pfn_to_pdx' (ffff82d0802d3270 != ffff82d080331e00)
>>> Duplicate symbol 'common.c#test_and_set_bit' (ffff82d0802d3360 != ffff82d080332250)
>>> Duplicate symbol 'common.c#variable_clear_bit' (ffff82d0802d2270 != ffff82d080337b50)
>>> Duplicate symbol 'compat.c#get_cpu_info' (ffff82d08026eab0 != ffff82d080200460)
>>> Duplicate symbol 'compat.c#get_cpu_info_from_stack' (ffff82d08026ebd0 != ffff82d080200f70)
>>> Duplicate symbol 'cpu_idle.c#get_cpu_info' (ffff82d0802ccb00 != ffff82d08035fcc0)
>>> [...]
>>>
>>> For the time being disable live-patching when building with clang,
>>> since duplicate symbols will trigger a build failure because
>>> ENFORCE_UNIQUE_SYMBOLS is now also enabled by default in conjunction
>>> with live-patching.
>>>
>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>
>> To be honest, as indicated before I'm inclined to nak this patch
>> on the basis that a proper solution has been posted almost 3 weeks
>> ago (and this was already v2).
> 
> What's that patch waiting on?

x86 and release acks.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH for-4.13] clang: do not enable live-patching support

[Jürgen Groß]

On 27.11.19 17:25, Jan Beulich wrote:
> On 27.11.2019 17:21, George Dunlap wrote:
>> On 11/27/19 4:14 PM, Jan Beulich wrote:
>>> On 27.11.2019 17:01, Roger Pau Monne wrote:
>>>> Live-patching requires unique symbols, and sadly the clang build
>>>> generates a lot of duplicate symbols:
>>>>
>>>> Duplicate symbol 'asid.c#get_cpu_info' (ffff82d0803032c0 != ffff82d0802e0f50)
>>>> Duplicate symbol 'asid.c#get_cpu_info_from_stack' (ffff82d0802e1080 != ffff82d0803032f0)
>>>> Duplicate symbol 'ats.c#__list_add' (ffff82d080260a00 != ffff82d080267c70)
>>>> Duplicate symbol 'boot.c#constant_test_bit' (ffff82d08040ea60 != ffff82d0804372f0)
>>>> Duplicate symbol 'common.c#clear_bit' (ffff82d080332440 != ffff82d0802d33b0)
>>>> Duplicate symbol 'common.c#constant_test_bit' (ffff82d080332340 != ffff82d0802d2220)
>>>> Duplicate symbol 'common.c#cpumask_check' (ffff82d0802d3370 != ffff82d080337b60)
>>>> Duplicate symbol 'common.c#get_cpu_info' (ffff82d0802d22b0 != ffff82d080331590)
>>>> Duplicate symbol 'common.c#get_cpu_info_from_stack' (ffff82d0802d31c0 != ffff82d0803374b0)
>>>> Duplicate symbol 'common.c#pfn_to_pdx' (ffff82d0802d3270 != ffff82d080331e00)
>>>> Duplicate symbol 'common.c#test_and_set_bit' (ffff82d0802d3360 != ffff82d080332250)
>>>> Duplicate symbol 'common.c#variable_clear_bit' (ffff82d0802d2270 != ffff82d080337b50)
>>>> Duplicate symbol 'compat.c#get_cpu_info' (ffff82d08026eab0 != ffff82d080200460)
>>>> Duplicate symbol 'compat.c#get_cpu_info_from_stack' (ffff82d08026ebd0 != ffff82d080200f70)
>>>> Duplicate symbol 'cpu_idle.c#get_cpu_info' (ffff82d0802ccb00 != ffff82d08035fcc0)
>>>> [...]
>>>>
>>>> For the time being disable live-patching when building with clang,
>>>> since duplicate symbols will trigger a build failure because
>>>> ENFORCE_UNIQUE_SYMBOLS is now also enabled by default in conjunction
>>>> with live-patching.
>>>>
>>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>>
>>> To be honest, as indicated before I'm inclined to nak this patch
>>> on the basis that a proper solution has been posted almost 3 weeks
>>> ago (and this was already v2).
>>
>> What's that patch waiting on?
> 
> x86 and release acks.

I plan to release ack the patch in case the missing maintainer's acks
are not coming in too late.


Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH for-4.13] clang: do not enable live-patching support

[George Dunlap]

On 11/27/19 4:35 PM, Jürgen Groß wrote:
> On 27.11.19 17:25, Jan Beulich wrote:
>> On 27.11.2019 17:21, George Dunlap wrote:
>>> On 11/27/19 4:14 PM, Jan Beulich wrote:
>>>> On 27.11.2019 17:01, Roger Pau Monne wrote:
>>>>> Live-patching requires unique symbols, and sadly the clang build
>>>>> generates a lot of duplicate symbols:
>>>>>
>>>>> Duplicate symbol 'asid.c#get_cpu_info' (ffff82d0803032c0 !=
>>>>> ffff82d0802e0f50)
>>>>> Duplicate symbol 'asid.c#get_cpu_info_from_stack' (ffff82d0802e1080
>>>>> != ffff82d0803032f0)
>>>>> Duplicate symbol 'ats.c#__list_add' (ffff82d080260a00 !=
>>>>> ffff82d080267c70)
>>>>> Duplicate symbol 'boot.c#constant_test_bit' (ffff82d08040ea60 !=
>>>>> ffff82d0804372f0)
>>>>> Duplicate symbol 'common.c#clear_bit' (ffff82d080332440 !=
>>>>> ffff82d0802d33b0)
>>>>> Duplicate symbol 'common.c#constant_test_bit' (ffff82d080332340 !=
>>>>> ffff82d0802d2220)
>>>>> Duplicate symbol 'common.c#cpumask_check' (ffff82d0802d3370 !=
>>>>> ffff82d080337b60)
>>>>> Duplicate symbol 'common.c#get_cpu_info' (ffff82d0802d22b0 !=
>>>>> ffff82d080331590)
>>>>> Duplicate symbol 'common.c#get_cpu_info_from_stack'
>>>>> (ffff82d0802d31c0 != ffff82d0803374b0)
>>>>> Duplicate symbol 'common.c#pfn_to_pdx' (ffff82d0802d3270 !=
>>>>> ffff82d080331e00)
>>>>> Duplicate symbol 'common.c#test_and_set_bit' (ffff82d0802d3360 !=
>>>>> ffff82d080332250)
>>>>> Duplicate symbol 'common.c#variable_clear_bit' (ffff82d0802d2270 !=
>>>>> ffff82d080337b50)
>>>>> Duplicate symbol 'compat.c#get_cpu_info' (ffff82d08026eab0 !=
>>>>> ffff82d080200460)
>>>>> Duplicate symbol 'compat.c#get_cpu_info_from_stack'
>>>>> (ffff82d08026ebd0 != ffff82d080200f70)
>>>>> Duplicate symbol 'cpu_idle.c#get_cpu_info' (ffff82d0802ccb00 !=
>>>>> ffff82d08035fcc0)
>>>>> [...]
>>>>>
>>>>> For the time being disable live-patching when building with clang,
>>>>> since duplicate symbols will trigger a build failure because
>>>>> ENFORCE_UNIQUE_SYMBOLS is now also enabled by default in conjunction
>>>>> with live-patching.
>>>>>
>>>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>>>
>>>> To be honest, as indicated before I'm inclined to nak this patch
>>>> on the basis that a proper solution has been posted almost 3 weeks
>>>> ago (and this was already v2).
>>>
>>> What's that patch waiting on?
>>
>> x86 and release acks.
> 
> I plan to release ack the patch in case the missing maintainer's acks
> are not coming in too late.

I think Andy's objection was that there has been zero testing of
livepatching on gcc.  Maybe we can find someone to do a smoke-test.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH for-4.13] clang: do not enable live-patching support

[Konrad Rzeszutek Wilk]

> > I plan to release ack the patch in case the missing maintainer's acks
> > are not coming in too late.
> 
> I think Andy's objection was that there has been zero testing of
> livepatching on gcc.  Maybe we can find someone to do a smoke-test.

As in integrate livepatch-build tools in osstest smoke-tests?
Because the livepatch test cases are in osstest, unless something went awry?

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH for-4.13] clang: do not enable live-patching support

[Andrew Cooper]

On 02/12/2019 15:53, Konrad Rzeszutek Wilk wrote:
>>> I plan to release ack the patch in case the missing maintainer's acks
>>> are not coming in too late.
>> I think Andy's objection was that there has been zero testing of
>> livepatching on gcc.  Maybe we can find someone to do a smoke-test.
> As in integrate livepatch-build tools in osstest smoke-tests?
> Because the livepatch test cases are in osstest, unless something went awry?

The sum total of livepatch testing in OSSTest is using the hand-coded
ELF objects from the tests/ directory.

This is perhaps ok for the basic mechanism, but its not representative
of actually building real livepatches using livepatch build tools.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH for-4.13] clang: do not enable live-patching support

[Konrad Rzeszutek Wilk]

On Mon, Dec 02, 2019 at 03:55:04PM +0000, Andrew Cooper wrote:
> On 02/12/2019 15:53, Konrad Rzeszutek Wilk wrote:
> >>> I plan to release ack the patch in case the missing maintainer's acks
> >>> are not coming in too late.
> >> I think Andy's objection was that there has been zero testing of
> >> livepatching on gcc.  Maybe we can find someone to do a smoke-test.
> > As in integrate livepatch-build tools in osstest smoke-tests?
> > Because the livepatch test cases are in osstest, unless something went awry?
> 
> The sum total of livepatch testing in OSSTest is using the hand-coded
> ELF objects from the tests/ directory.
> 
> This is perhaps ok for the basic mechanism, but its not representative
> of actually building real livepatches using livepatch build tools.

True. But it tests the _hypervisor_ livepatch code.

I am thinking that this discussion about "oh, but livepatch-build tools don't work b/c"
is well <shrug> sucks but should never block an release as the core
livepatch functionality is OK.

Irrespective of that the testing of livepatch-build tools should be in osstest,
granted nobody has taken a step in this - but is somebody signing up for it?
[I can't, -ENOTIME]
> 
> ~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH for-4.13] clang: do not enable live-patching support

[George Dunlap]

> On Dec 2, 2019, at 5:01 PM, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> 
> On Mon, Dec 02, 2019 at 03:55:04PM +0000, Andrew Cooper wrote:
>> On 02/12/2019 15:53, Konrad Rzeszutek Wilk wrote:
>>>>> I plan to release ack the patch in case the missing maintainer's acks
>>>>> are not coming in too late.
>>>> I think Andy's objection was that there has been zero testing of
>>>> livepatching on gcc.  Maybe we can find someone to do a smoke-test.
>>> As in integrate livepatch-build tools in osstest smoke-tests?
>>> Because the livepatch test cases are in osstest, unless something went awry?
>> 
>> The sum total of livepatch testing in OSSTest is using the hand-coded
>> ELF objects from the tests/ directory.
>> 
>> This is perhaps ok for the basic mechanism, but its not representative
>> of actually building real livepatches using livepatch build tools.
> 
> True. But it tests the _hypervisor_ livepatch code.
> 
> I am thinking that this discussion about "oh, but livepatch-build tools don't work b/c"
> is well <shrug> sucks but should never block an release as the core
> livepatch functionality is OK.

I think a parallel is if Xen doesn’t build with a particular version of the compiler, or can’t build on a particular distro for some reason.  We should certainly *try* to make things work with other projects, but if the issue is clearly with the other project, we shouldn’t have to block to wait for that other project to get things sorted out.

-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH for-4.13] clang: do not enable live-patching support

[Andrew Cooper]

On 03/12/2019 09:17, George Dunlap wrote:
>
>> On Dec 2, 2019, at 5:01 PM, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
>>
>> On Mon, Dec 02, 2019 at 03:55:04PM +0000, Andrew Cooper wrote:
>>> On 02/12/2019 15:53, Konrad Rzeszutek Wilk wrote:
>>>>>> I plan to release ack the patch in case the missing maintainer's acks
>>>>>> are not coming in too late.
>>>>> I think Andy's objection was that there has been zero testing of
>>>>> livepatching on gcc.  Maybe we can find someone to do a smoke-test.
>>>> As in integrate livepatch-build tools in osstest smoke-tests?
>>>> Because the livepatch test cases are in osstest, unless something went awry?
>>> The sum total of livepatch testing in OSSTest is using the hand-coded
>>> ELF objects from the tests/ directory.
>>>
>>> This is perhaps ok for the basic mechanism, but its not representative
>>> of actually building real livepatches using livepatch build tools.
>> True. But it tests the _hypervisor_ livepatch code.
>>
>> I am thinking that this discussion about "oh, but livepatch-build tools don't work b/c"
>> is well <shrug> sucks but should never block an release as the core
>> livepatch functionality is OK.
> I think a parallel is if Xen doesn’t build with a particular version of the compiler, or can’t build on a particular distro for some reason.  We should certainly *try* to make things work with other projects, but if the issue is clearly with the other project, we shouldn’t have to block to wait for that other project to get things sorted out.

This isn't a valid comparison.

livepatch-build-tools is a concrete thing, built and maintained by us
(the Xen community), explicitly for the purpose generating livepatches
between two versions of Xen.  It lives at
https://xenbits.xen.org/gitweb/?p=livepatch-build-tools.git;a=summary on
xenbits, just like xen.git.

It *should* be used in OSSTest, have a push gate, and block breaking
changes either to Xen or to the tools themselves, before the breaking
changes get accepted into master of either repo.

Otherwise, the support statement for livepatching needs to change to be
"we don't bother testing the two parts of this supported feature together".

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH for-4.13] clang: do not enable live-patching support

[Lars Kurth]

[-- Attachment #1.1: Type: text/plain, Size: 2712 bytes --]



> On 6 Dec 2019, at 14:21, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> 
> On 03/12/2019 09:17, George Dunlap wrote:
>> 
>>> On Dec 2, 2019, at 5:01 PM, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
>>> 
>>> On Mon, Dec 02, 2019 at 03:55:04PM +0000, Andrew Cooper wrote:
>>>> On 02/12/2019 15:53, Konrad Rzeszutek Wilk wrote:
>>>>>>> I plan to release ack the patch in case the missing maintainer's acks
>>>>>>> are not coming in too late.
>>>>>> I think Andy's objection was that there has been zero testing of
>>>>>> livepatching on gcc.  Maybe we can find someone to do a smoke-test.
>>>>> As in integrate livepatch-build tools in osstest smoke-tests?
>>>>> Because the livepatch test cases are in osstest, unless something went awry?
>>>> The sum total of livepatch testing in OSSTest is using the hand-coded
>>>> ELF objects from the tests/ directory.
>>>> 
>>>> This is perhaps ok for the basic mechanism, but its not representative
>>>> of actually building real livepatches using livepatch build tools.
>>> True. But it tests the _hypervisor_ livepatch code.
>>> 
>>> I am thinking that this discussion about "oh, but livepatch-build tools don't work b/c"
>>> is well <shrug> sucks but should never block an release as the core
>>> livepatch functionality is OK.
>> I think a parallel is if Xen doesn’t build with a particular version of the compiler, or can’t build on a particular distro for some reason.  We should certainly *try* to make things work with other projects, but if the issue is clearly with the other project, we shouldn’t have to block to wait for that other project to get things sorted out.
> 
> This isn't a valid comparison.
> 
> livepatch-build-tools is a concrete thing, built and maintained by us
> (the Xen community), explicitly for the purpose generating livepatches
> between two versions of Xen.  It lives at
> https://xenbits.xen.org/gitweb/?p=livepatch-build-tools.git;a=summary <https://xenbits.xen.org/gitweb/?p=livepatch-build-tools.git;a=summary> on
> xenbits, just like xen.git.


First a couple of questions: I noticed that neither Ross to xen-devel is on this thread

I agree with Andy: we got away lucky so far, as there have been few changes to the live patch-build-tools


> It *should* be used in OSSTest, have a push gate, and block breaking
> changes either to Xen or to the tools themselves, before the breaking
> changes get accepted into master of either repo.

Although I agree with you, we should not block 4.13 for it and do some manual testing for this release
But we should have a plan in place for 4.14 to address this and maybe agree to block 4.14 if that has not happened

Lars

[-- Attachment #1.2: Type: text/html, Size: 12591 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel