On 08/04/20 08:09, Cameron Esfahani wrote:
>>>
>>> if (efer & MSR_EFER_LME) {
>>> - if (!(old_cr0 & CR0_PG) && (cr0 & CR0_PG)) {
>>> + if (!(old_cr0 & CR0_PG_MASK) && (cr0 & CR0_PG_MASK)) {
>>> enter_long_mode(vcpu, cr0, efer);
>>> }
>>> - if (/*(old_cr0 & CR0_PG) &&*/ !(cr0 & CR0_PG)) {
>>> + if (!(cr0 & CR0_PG_MASK)) {
>> IMO the patch should only change CR0_PG to CR0_PG_MASK without removal
>> of the commented condition.
>>
>> In the next patch you're improving how long mode exit is done and
>> replacement of the comment with an implementation fits better there.
>>
> The reason I removed that code was because checkpatch.pl scolded me for a
> patch with code commented out.
>
> I assumed that I'd get a similar warning from patchew.org about some
> erroneous coding styles.
>
> So I thought the easiest thing would be to remove that code as well.
>
> But I'll defer to you or Paolo: should I remove that commented code with this
> patch?
checkpatch errors are not absolutely a no-no, especially if the code is
pre-existing and/or it goes away later in the patch. In this case,
since you have already written the patch it's okay to keep it as is.
Paolo
