public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Alexander Duyck <alexander.duyck@gmail.com>
To: "Li, ZhenHua" <zhen-hual@hp.com>, Dave Young <dyoung@redhat.com>
Cc: dwmw2@infradead.org, indou.takao@jp.fujitsu.com, bhe@redhat.com,
	joro@8bytes.org, vgoyal@redhat.com,
	iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
	linux-pci@vger.kernel.org, kexec@lists.infradead.org,
	alex.williamson@redhat.com, ddutile@redhat.com,
	ishii.hironobu@jp.fujitsu.com, bhelgaas@google.com,
	doug.hatch@hp.com, jerry.hoemann@hp.com, tom.vaden@hp.com,
	li.zhang6@hp.com, lisa.mitchell@hp.com,
	billsumnerlinux@gmail.com, rwright@hp.com
Subject: Re: [PATCH v11 02/10] iommu/vt-d: Items required for kdump
Date: Tue, 12 May 2015 23:31:44 -0700	[thread overview]
Message-ID: <5552EFD0.3020307@gmail.com> (raw)
In-Reply-To: <5552ACB9.5050309@hp.com>

On 05/12/2015 06:45 PM, Li, ZhenHua wrote:
> On 05/12/2015 04:17 PM, Dave Young wrote:
>> On 05/11/15 at 05:52pm, Li, Zhen-Hua wrote:
>>> Add context entry functions needed for kdump.
>>> +/*
>>> + * Fix Crashdump failure caused by leftover DMA through a hardware
>>> IOMMU
>>> + *
>>> + * Fixes the crashdump kernel to deal with an active iommu and legacy
>>> + * DMA from the (old) panicked kernel in a manner similar to how legacy
>>> + * DMA is handled when no hardware iommu was in use by the old
>>> kernel --
>>> + * allow the legacy DMA to continue into its current buffers.
>>> + *
>>> + * In the crashdump kernel, this code:
>>> + * 1. skips disabling the IOMMU's translating.
>>> + * 2. Do not re-enable IOMMU's translating.
>>> + * 3. In kdump kernel, use the old root entry table.
>>> + * 4. Allocate pages for new context entry, copy data from old
>>> context entries
>>> + *    in the old kernel to the new ones.
>>> + *
>>> + * In other kinds of kernel, for example, a kernel started by kexec,
>>> + * do the same thing as crashdump kernel.
>>> + */
>>> +
>>> +
>>
>> Above comments should come along with the code changes instead of
>> putting it
>> in this patch.
>>
>> BTW, there's one more blank line at the end..
> Code change is in 00/10, the cover letter.
> And the blank does not matter, I checked the patch with
> scripts/checkpatch.pl,  no warnings, no errors.

I agree with Dave.  The comment by itself makes no sense.  You would be 
better off moving this to patch 3 where you actually start to add code.

You remove the double space in patch 5 of your set so it must of 
triggered something.

Neither of these are grounds for rejecting the patches, but it is 
something you may want to clean up if you end up resubmitting.

- Alex


  reply	other threads:[~2015-05-13  6:31 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-11  9:52 [PATCH v11 0/10] iommu/vt-d: Fix intel vt-d faults in kdump kernel Li, Zhen-Hua
2015-05-11  9:52 ` [PATCH v11 01/10] iommu/vt-d: New function to attach domain with id Li, Zhen-Hua
2015-05-11  9:52 ` [PATCH v11 02/10] iommu/vt-d: Items required for kdump Li, Zhen-Hua
2015-05-12  8:17   ` Dave Young
2015-05-13  1:45     ` Li, ZhenHua
2015-05-13  6:31       ` Alexander Duyck [this message]
2015-05-13  8:42       ` Dave Young
2015-05-11  9:52 ` [PATCH v11 03/10] iommu/vt-d: Function to get existing context entry Li, Zhen-Hua
2015-05-11  9:52 ` [PATCH v11 04/10] iommu/vt-d: functions to copy data from old mem Li, Zhen-Hua
2015-05-12  8:29   ` Dave Young
2015-05-13  9:00   ` Baoquan He
2015-05-13  9:13     ` Li, ZhenHua
2015-05-13  9:21       ` Baoquan He
2015-06-08 14:15   ` David Woodhouse
2015-06-08 15:21     ` Joerg Roedel
2015-06-08 15:44       ` David Woodhouse
2015-05-11  9:52 ` [PATCH v11 05/10] iommu/vt-d: Add functions to load and save old re Li, Zhen-Hua
2015-05-12  8:37   ` Dave Young
2015-05-13  1:47     ` Li, ZhenHua
2015-05-13  8:49       ` Dave Young
2015-05-11  9:52 ` [PATCH v11 06/10] iommu/vt-d: datatypes and functions used for kdump Li, Zhen-Hua
2015-05-12  8:48   ` Dave Young
2015-05-13  8:56   ` Baoquan He
2015-05-13  8:58     ` Li, ZhenHua
2015-05-13  9:10       ` Baoquan He
2015-05-11  9:52 ` [PATCH v11 07/10] iommu/vt-d: enable kdump support in iommu module Li, Zhen-Hua
2015-05-12  8:52   ` Dave Young
2015-05-13  2:10   ` Baoquan He
2015-05-13  2:28     ` Li, ZhenHua
2015-05-13  2:36       ` Baoquan He
2015-05-11  9:52 ` [PATCH v11 08/10] iommu/vt-d: assign new page table for dma_map Li, Zhen-Hua
2015-05-20 23:52   ` Baoquan He
2015-05-21  1:27     ` Li, ZhenHua
2015-05-21  6:54       ` Baoquan He
2015-05-21  8:40         ` Li, ZhenHua
2015-05-21 10:11           ` Baoquan He
2015-05-11  9:52 ` [PATCH v11 09/10] iommu/vt-d: Copy functions for irte Li, Zhen-Hua
2015-05-12  9:00   ` Dave Young
2015-05-11  9:52 ` [PATCH v11 10/10] iommu/vt-d: Use old irte in kdump kernel Li, Zhen-Hua
2015-05-12  6:18 ` [PATCH v11 0/10] iommu/vt-d: Fix intel vt-d faults " Baoquan He
2015-05-12  9:04 ` Dave Young
2015-05-12  9:34 ` Li, Zhen-Hua
2015-05-13  1:54 ` Li, ZhenHua
2015-05-18 10:05   ` Li, ZhenHua
2015-05-19  1:13     ` Dave Young
2015-05-19  7:43       ` Li, ZhenHua
2015-05-29 16:21 ` Joerg Roedel
2015-05-30 11:23   ` Li, Zhen-Hua
2015-06-08 14:26 ` David Woodhouse
2015-06-08 15:29   ` Joerg Roedel
2015-06-08 15:50     ` David Woodhouse
2015-06-08 16:13       ` Joerg Roedel
2015-06-09 12:55         ` David Woodhouse
2015-06-10  9:21           ` Joerg Roedel
2015-06-10  9:32             ` Li, ZhenHua
2015-06-10 14:10               ` David Woodhouse

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5552EFD0.3020307@gmail.com \
    --to=alexander.duyck@gmail.com \
    --cc=alex.williamson@redhat.com \
    --cc=bhe@redhat.com \
    --cc=bhelgaas@google.com \
    --cc=billsumnerlinux@gmail.com \
    --cc=ddutile@redhat.com \
    --cc=doug.hatch@hp.com \
    --cc=dwmw2@infradead.org \
    --cc=dyoung@redhat.com \
    --cc=indou.takao@jp.fujitsu.com \
    --cc=iommu@lists.linux-foundation.org \
    --cc=ishii.hironobu@jp.fujitsu.com \
    --cc=jerry.hoemann@hp.com \
    --cc=joro@8bytes.org \
    --cc=kexec@lists.infradead.org \
    --cc=li.zhang6@hp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lisa.mitchell@hp.com \
    --cc=rwright@hp.com \
    --cc=tom.vaden@hp.com \
    --cc=vgoyal@redhat.com \
    --cc=zhen-hual@hp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox