Re: [Xen-devel] [PATCH Altp2m cleanup v4 2/4] altp2m cleanup work

[George Dunlap]

On 13/09/16 18:38, Lai, Paul wrote:
> On Mon, Sep 12, 2016 at 11:47:35AM +0100, George Dunlap wrote:
>> On 08/09/16 17:45, Lai, Paul C wrote:
>>> [Paul2] in-line
>>
>> If you're going to engage in discussions on xen-devel it would really be
>> worth your time to find a mail setup that allows you to actually quote
>> properly such that you can reply in-line without these manual mark-ups.
>>
>> If you can't configure your mail reader to do proper nested quoting, and
>> you can't / don't want to change your mail reader, then you could
>> consider doing what I do and signing up for a gmail account to get all
>> the xen-devel mail and replying from there.
>>
>> Thanks,
>>  -George
>>
> 
> George,
> Thanks for the suggestions.

No problem -- and I hope the tone of my message didn't come off too
wrong.  I was trying to find solutions that work for everyone, not
criticize. :-)

 -George


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

Re: [Xen-devel] [PATCH Altp2m cleanup v4 2/4] altp2m cleanup work

[Lai, Paul]

On Mon, Sep 12, 2016 at 11:47:35AM +0100, George Dunlap wrote:
> On 08/09/16 17:45, Lai, Paul C wrote:
> > [Paul2] in-line
> 
> If you're going to engage in discussions on xen-devel it would really be
> worth your time to find a mail setup that allows you to actually quote
> properly such that you can reply in-line without these manual mark-ups.
> 
> If you can't configure your mail reader to do proper nested quoting, and
> you can't / don't want to change your mail reader, then you could
> consider doing what I do and signing up for a gmail account to get all
> the xen-devel mail and replying from there.
> 
> Thanks,
>  -George
> 

George,
Thanks for the suggestions.
Hopefully this reply illustrates that I can learn.

-Paul

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

Re: [Xen-devel] [PATCH Altp2m cleanup v4 2/4] altp2m cleanup work

[George Dunlap]

On 08/09/16 17:45, Lai, Paul C wrote:
> [Paul2] in-line

If you're going to engage in discussions on xen-devel it would really be
worth your time to find a mail setup that allows you to actually quote
properly such that you can reply in-line without these manual mark-ups.

If you can't configure your mail reader to do proper nested quoting, and
you can't / don't want to change your mail reader, then you could
consider doing what I do and signing up for a gmail account to get all
the xen-devel mail and replying from there.

Thanks,
 -George


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

Re: [Xen-devel] [PATCH Altp2m cleanup v4 2/4] altp2m cleanup work

[Lai, Paul C]

[Paul2] in-line

-Original Message-
From: Tamas K Lengyel [mailto:tamas.k.leng...@gmail.com] 
Sent: Thursday, September 8, 2016 9:07 AM
To: Lai, Paul C <paul.c@intel.com>
Cc: Jan Beulich <jbeul...@suse.com>; xen-devel 
<xen-de...@lists.xenproject.org>; Sahita, Ravi <ravi.sah...@intel.com>; 
george.dun...@citrix.com
Subject: Re: [Xen-devel] [PATCH Altp2m cleanup v4 2/4] altp2m cleanup work

On Thu, Sep 8, 2016 at 9:50 AM, Lai, Paul C <paul.c@intel.com> wrote:
> [Paul] in-line
>
> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: Thursday, September 8, 2016 7:47 AM
> To: Lai, Paul C <paul.c@intel.com>
> Cc: george.dun...@citrix.com; Sahita, Ravi <ravi.sah...@intel.com>; 
> xen-devel <xen-de...@lists.xenproject.org>
> Subject: Re: [PATCH Altp2m cleanup v4 2/4] altp2m cleanup work
>
>>>> On 08.09.16 at 00:04, <paul.c@intel.com> wrote:
>> Indent goto labels by one space
>> Inline (header) altp2m functions
>> Define default behavior in switch
>> Define max and min for range of altp2m macroed values
>>
>> Signed-off-by: Paul Lai <paul.c@intel.com>
>> ---
>
> Missing a brief summary of changes from previous version here.
>
>> @@ -5413,6 +5426,8 @@ static int do_altp2m_op(
>>  rc = p2m_change_altp2m_gfn(d, a.u.change_gfn.view,
>>  _gfn(a.u.change_gfn.old_gfn),
>>  _gfn(a.u.change_gfn.new_gfn));
>> +default:
>> +ASSERT_UNREACHABLE();
>>  }
>
> Did you test anything using HVMOP_altp2m_change_gfn with this change, on a 
> debug build? There's obviously an unintended fall- through right now.
>
> [Paul] - Yes, this patch series was tested with Tamas's 
> HVMOP_altp2m_change_gfn in a debug build.

Not sure what you used here, the xen-access tool right now in Xen doesn't (yet) 
exercise the gfn remapping, only the mem-access parts.
Sergej has a patch for this in the arm-altp2m series though.

Tamas

[Paul2] All I was testing was if xen-access behaved as before the gfn remapping 
patch was introduced.  After the gfn remapping patch was introduced, 'xen-acess 
-m 1 altp2m_write' hanged the system.  Now, with your fix, it doesn't and the 
output of xen-access appears as before.   Thanks, -Paul
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH Altp2m cleanup v4 2/4] altp2m cleanup work

[Tamas K Lengyel]

On Thu, Sep 8, 2016 at 9:50 AM, Lai, Paul C  wrote:
> [Paul] in-line
>
> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: Thursday, September 8, 2016 7:47 AM
> To: Lai, Paul C 
> Cc: george.dun...@citrix.com; Sahita, Ravi ; xen-devel 
> 
> Subject: Re: [PATCH Altp2m cleanup v4 2/4] altp2m cleanup work
>
 On 08.09.16 at 00:04,  wrote:
>> Indent goto labels by one space
>> Inline (header) altp2m functions
>> Define default behavior in switch
>> Define max and min for range of altp2m macroed values
>>
>> Signed-off-by: Paul Lai 
>> ---
>
> Missing a brief summary of changes from previous version here.
>
>> @@ -5413,6 +5426,8 @@ static int do_altp2m_op(
>>  rc = p2m_change_altp2m_gfn(d, a.u.change_gfn.view,
>>  _gfn(a.u.change_gfn.old_gfn),
>>  _gfn(a.u.change_gfn.new_gfn));
>> +default:
>> +ASSERT_UNREACHABLE();
>>  }
>
> Did you test anything using HVMOP_altp2m_change_gfn with this change, on a 
> debug build? There's obviously an unintended fall- through right now.
>
> [Paul] - Yes, this patch series was tested with Tamas's 
> HVMOP_altp2m_change_gfn in a debug build.

Not sure what you used here, the xen-access tool right now in Xen
doesn't (yet) exercise the gfn remapping, only the mem-access parts.
Sergej has a patch for this in the arm-altp2m series though.

Tamas

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

Re: [Xen-devel] [PATCH Altp2m cleanup v4 2/4] altp2m cleanup work

[Lai, Paul C]

[Paul] in-line

-Original Message-
From: Jan Beulich [mailto:jbeul...@suse.com] 
Sent: Thursday, September 8, 2016 7:47 AM
To: Lai, Paul C 
Cc: george.dun...@citrix.com; Sahita, Ravi ; xen-devel 

Subject: Re: [PATCH Altp2m cleanup v4 2/4] altp2m cleanup work

>>> On 08.09.16 at 00:04,  wrote:
> Indent goto labels by one space
> Inline (header) altp2m functions
> Define default behavior in switch
> Define max and min for range of altp2m macroed values
> 
> Signed-off-by: Paul Lai 
> ---

Missing a brief summary of changes from previous version here.

> @@ -5413,6 +5426,8 @@ static int do_altp2m_op(
>  rc = p2m_change_altp2m_gfn(d, a.u.change_gfn.view,
>  _gfn(a.u.change_gfn.old_gfn),
>  _gfn(a.u.change_gfn.new_gfn));
> +default:
> +ASSERT_UNREACHABLE();
>  }

Did you test anything using HVMOP_altp2m_change_gfn with this change, on a 
debug build? There's obviously an unintended fall- through right now.

[Paul] - Yes, this patch series was tested with Tamas's HVMOP_altp2m_change_gfn 
in a debug build.


>  /* emulates #VE */
> -bool_t altp2m_vcpu_emulate_ve(struct vcpu *v);
> +static inline bool_t altp2m_vcpu_emulate_ve(struct vcpu *v)

Already on v3 I had asked to switch to plain bool (and true/false as 
appropriate).

[Paul] - Maybe I misunderstood something here.  I fixed the return value to 
false in the patch.   ... Ah, it's the non-false case that's returning void.  
Will fix.

Jan


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

Re: [Xen-devel] [PATCH Altp2m cleanup v4 2/4] altp2m cleanup work

[Jan Beulich]

>>> On 08.09.16 at 00:04,  wrote:
> Indent goto labels by one space
> Inline (header) altp2m functions
> Define default behavior in switch
> Define max and min for range of altp2m macroed values
> 
> Signed-off-by: Paul Lai 
> ---

Missing a brief summary of changes from previous version here.

> @@ -5413,6 +5426,8 @@ static int do_altp2m_op(
>  rc = p2m_change_altp2m_gfn(d, a.u.change_gfn.view,
>  _gfn(a.u.change_gfn.old_gfn),
>  _gfn(a.u.change_gfn.new_gfn));
> +default:
> +ASSERT_UNREACHABLE();
>  }

Did you test anything using HVMOP_altp2m_change_gfn with this
change, on a debug build? There's obviously an unintended fall-
through right now.

>  /* emulates #VE */
> -bool_t altp2m_vcpu_emulate_ve(struct vcpu *v);
> +static inline bool_t altp2m_vcpu_emulate_ve(struct vcpu *v)

Already on v3 I had asked to switch to plain bool (and true/false as
appropriate).

Jan


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