From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Chen, Tiejun" <tiejun.chen@intel.com>
Subject: Re: [v8][PATCH 06/16] hvmloader/pci: disable all pci
 devices conflicting with rdm
Date: Thu, 16 Jul 2015 23:20:26 +0800
Message-ID: <55A7CBBA.90606@intel.com>
References: <1437029582-19564-1-git-send-email-tiejun.chen@intel.com>
	<1437029582-19564-7-git-send-email-tiejun.chen@intel.com>
	<CAFLBxZZhDHi2XQ9UYkt6gfEiVMNRGxxer3zNptFGVsns2OYOfw@mail.gmail.com>
	<55A79AFA.3040500@intel.com>
	<CAFLBxZY86xDsNKO9zHMouk+sEPuCcYaR_49TQ87zjXxtBPqKnQ@mail.gmail.com>
	<55A7AFF4.1040909@intel.com>
	<55A7CE890200007800091EC8@mail.emea.novell.com>
	<55A7B647.8080808@intel.com>
	<55A7E1C30200007800091F48@mail.emea.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <55A7E1C30200007800091F48@mail.emea.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: Wei Liu <wei.liu2@citrix.com>, Ian Campbell <ian.campbell@citrix.com>, Stefano Stabellini <stefano.stabellini@eu.citrix.com>, George Dunlap <George.Dunlap@eu.citrix.com>, Andrew Cooper <andrew.cooper3@citrix.com>, Ian Jackson <ian.jackson@eu.citrix.com>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>, Keir Fraser <keir@xen.org>
List-Id: xen-devel@lists.xenproject.org

>> What about this?
>
> Looks reasonable (but don't forget that I continue to be unconvinced
> that the patch as a whole makes sense).

Yes, I always keep this in my mind as I mentioned in patch #00. Any risk 
you're still concerning? Is it that case if guest OS force enabling 
these devices again? IMO, at this point there are two cases:

#1. Without passing through a RMRR device

Those emulated devices don't create 1:1 mapping so its safe, right?

#2. With passing through a RMRR device

This just probably cause these associated devices not to work well, but 
still don't bring any impact to other Domains, right? I mean this isn't 
going to worsen the preexisting situation.

If I'm wrong please correct me.

Thanks
Tiejun

>
> Jan
>
>> +    bool is_conflict = false;
>>
>>        for ( devfn = 0; devfn < 256; devfn++ )
>>        {
>> @@ -60,7 +61,7 @@ static void disable_conflicting_devices(void)
>>                continue;
>>
>>            /* Check all bars */
>> -        for ( bar = 0; bar < 7; bar++ )
>> +        for ( bar = 0; bar < 7 && !is_conflict; bar++ )
>>            {
>>                bar_reg = PCI_BASE_ADDRESS_0 + 4*bar;
>>                if ( bar == 6 )
>> @@ -89,7 +90,7 @@ static void disable_conflicting_devices(void)
>>                bar_sz = pci_readl(devfn, bar_reg);
>>                bar_sz &= PCI_BASE_ADDRESS_MEM_MASK;
>>
>> -            for ( i = 0; i < memory_map.nr_map ; i++ )
>> +            for ( i = 0; i < memory_map.nr_map && !is_conflict; i++ )
>>                {
>>                    if ( memory_map.map[i].type == E820_RESERVED )
>>                    {
>> @@ -105,13 +106,13 @@ static void disable_conflicting_devices(void)
>>                                   devfn>>3, devfn&7, bar_reg, bar_data);
>>                            cmd = pci_readw(devfn, PCI_COMMAND);
>>                            pci_writew(devfn, PCI_COMMAND, ~cmd);
>> -                        /* Jump next device. */
>> -                        goto check_next_device;
>> +                        /* So need to jump next device. */
>> +                        is_conflict = true;
>>                        }
>>                    }
>>                }
>>            }
>> - check_next_device:
>> +        is_conflict = false;
>>        }
>>    }
>>
>> Thanks
>> Tiejun
>
>
>
>