All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
To: Igor Druzhinin <igor.druzhinin@citrix.com>,
	xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org
Cc: jgross@suse.com
Subject: Re: [Xen-devel] [PATCH] xen/pci: try to reserve MCFG areas earlier
Date: Thu, 12 Sep 2019 13:33:56 -0400	[thread overview]
Message-ID: <dd95aa25-bea7-dbc1-fea3-0e21d00d0686@oracle.com> (raw)
In-Reply-To: <74c9d2cc-a528-2cec-099e-0d803aeace6f@citrix.com>

On 9/10/19 9:15 PM, Igor Druzhinin wrote:
> On 10/09/2019 22:19, Boris Ostrovsky wrote:
>> On 9/10/19 4:36 PM, Igor Druzhinin wrote:
>>> On 10/09/2019 18:48, Boris Ostrovsky wrote:
>>>> On 9/10/19 5:46 AM, Igor Druzhinin wrote:
>>>>> On 10/09/2019 02:47, Boris Ostrovsky wrote:
>>>>>> On 9/9/19 5:48 PM, Igor Druzhinin wrote:
>>>>>>> On 09/09/2019 20:19, Boris Ostrovsky wrote:
>>>>>>>
>>>>>>>> The other question I have is why you think it's worth keeping
>>>>>>>> xen_mcfg_late() as a late initcall. How could MCFG info be updated
>>>>>>>> between acpi_init() and late_initcalls being run? I'd think it can only
>>>>>>>> happen when a new device is hotplugged.
>>>>>>>>
>>>>>>> It was a precaution against setup_mcfg_map() calls that might add new
>>>>>>> areas that are not in MCFG table but for some reason have _CBA method.
>>>>>>> It's obviously a "firmware is broken" scenario so I don't have strong
>>>>>>> feelings to keep it here. Will prefer to remove in v2 if you want.
>>>>>> Isn't setup_mcfg_map() called before the first xen_add_device() which is where you are calling xen_mcfg_late()?
>>>>>>
>>>>> setup_mcfg_map() calls are done in order of root bus discovery which
>>>>> happens *after* the previous root bus has been enumerated. So the order
>>>>> is: call setup_mcfg_map() for root bus 0, find that
>>>>> pci_mmcfg_late_init() has finished MCFG area registration, perform PCI
>>>>> enumeration of bus 0, call xen_add_device() for every device there, call
>>>>> setup_mcfg_map() for root bus X, etc.
>>>> Ah, yes. Multiple busses.
>>>>
>>>> If that's the case then why don't we need to call xen_mcfg_late() for
>>>> the first device on each bus?
>>>>
>>> Ideally, yes - we'd like to call it for every bus discovered. But boot
>>> time buses are already in MCFG (otherwise system boot might not simply
>>> work as Jan pointed out) so it's not strictly required. The only case is
>>> a potential PCI bus hot-plug but I'm not sure it actually works in
>>> practice and we certainly didn't support it before. It might be solved
>>> theoretically by subscribing to acpi_bus_type that is available after
>>> acpi_init().
>> OK. Then *I think* we can drop late_initcall() but I would really like
>> to hear when others think.

Since noone commented then can you send a v2 with second patch removing
the late call?

Also, in the first patch please limit the scope of pci_mcfg_reserved to
just xen_add_device().

-boris


>>
> Another thing that I implied by "not supporting" but want to explicitly
> call out is that currently Xen will refuse reserving any MCFG area
> unless it actually existed in MCFG table at boot. I don't clearly
> understand reasoning behind it but it might be worth relaxing at least
> size matching restriction on Xen side now with this change.
>
> Igor
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel


WARNING: multiple messages have this Message-ID (diff)
From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
To: Igor Druzhinin <igor.druzhinin@citrix.com>,
	xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org
Cc: jgross@suse.com
Subject: Re: [Xen-devel] [PATCH] xen/pci: try to reserve MCFG areas earlier
Date: Thu, 12 Sep 2019 13:33:56 -0400	[thread overview]
Message-ID: <dd95aa25-bea7-dbc1-fea3-0e21d00d0686@oracle.com> (raw)
In-Reply-To: <74c9d2cc-a528-2cec-099e-0d803aeace6f@citrix.com>

On 9/10/19 9:15 PM, Igor Druzhinin wrote:
> On 10/09/2019 22:19, Boris Ostrovsky wrote:
>> On 9/10/19 4:36 PM, Igor Druzhinin wrote:
>>> On 10/09/2019 18:48, Boris Ostrovsky wrote:
>>>> On 9/10/19 5:46 AM, Igor Druzhinin wrote:
>>>>> On 10/09/2019 02:47, Boris Ostrovsky wrote:
>>>>>> On 9/9/19 5:48 PM, Igor Druzhinin wrote:
>>>>>>> On 09/09/2019 20:19, Boris Ostrovsky wrote:
>>>>>>>
>>>>>>>> The other question I have is why you think it's worth keeping
>>>>>>>> xen_mcfg_late() as a late initcall. How could MCFG info be updated
>>>>>>>> between acpi_init() and late_initcalls being run? I'd think it can only
>>>>>>>> happen when a new device is hotplugged.
>>>>>>>>
>>>>>>> It was a precaution against setup_mcfg_map() calls that might add new
>>>>>>> areas that are not in MCFG table but for some reason have _CBA method.
>>>>>>> It's obviously a "firmware is broken" scenario so I don't have strong
>>>>>>> feelings to keep it here. Will prefer to remove in v2 if you want.
>>>>>> Isn't setup_mcfg_map() called before the first xen_add_device() which is where you are calling xen_mcfg_late()?
>>>>>>
>>>>> setup_mcfg_map() calls are done in order of root bus discovery which
>>>>> happens *after* the previous root bus has been enumerated. So the order
>>>>> is: call setup_mcfg_map() for root bus 0, find that
>>>>> pci_mmcfg_late_init() has finished MCFG area registration, perform PCI
>>>>> enumeration of bus 0, call xen_add_device() for every device there, call
>>>>> setup_mcfg_map() for root bus X, etc.
>>>> Ah, yes. Multiple busses.
>>>>
>>>> If that's the case then why don't we need to call xen_mcfg_late() for
>>>> the first device on each bus?
>>>>
>>> Ideally, yes - we'd like to call it for every bus discovered. But boot
>>> time buses are already in MCFG (otherwise system boot might not simply
>>> work as Jan pointed out) so it's not strictly required. The only case is
>>> a potential PCI bus hot-plug but I'm not sure it actually works in
>>> practice and we certainly didn't support it before. It might be solved
>>> theoretically by subscribing to acpi_bus_type that is available after
>>> acpi_init().
>> OK. Then *I think* we can drop late_initcall() but I would really like
>> to hear when others think.

Since noone commented then can you send a v2 with second patch removing
the late call?

Also, in the first patch please limit the scope of pci_mcfg_reserved to
just xen_add_device().

-boris


>>
> Another thing that I implied by "not supporting" but want to explicitly
> call out is that currently Xen will refuse reserving any MCFG area
> unless it actually existed in MCFG table at boot. I don't clearly
> understand reasoning behind it but it might be worth relaxing at least
> size matching restriction on Xen side now with this change.
>
> Igor
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel


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

  parent reply	other threads:[~2019-09-12 17:34 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-04  0:20 [PATCH] xen/pci: try to reserve MCFG areas earlier Igor Druzhinin
2019-09-04  0:20 ` [Xen-devel] " Igor Druzhinin
2019-09-04  9:08 ` Jan Beulich
2019-09-04  9:08   ` [Xen-devel] " Jan Beulich
2019-09-04 11:36   ` Igor Druzhinin
2019-09-04 11:36     ` [Xen-devel] " Igor Druzhinin
2019-09-04 12:09     ` Jan Beulich
2019-09-04 12:09       ` [Xen-devel] " Jan Beulich
2019-09-06 22:30 ` Boris Ostrovsky
2019-09-06 22:30   ` [Xen-devel] " Boris Ostrovsky
2019-09-06 23:00   ` Igor Druzhinin
2019-09-06 23:00     ` [Xen-devel] " Igor Druzhinin
2019-09-08 18:28     ` Boris Ostrovsky
2019-09-08 18:28       ` [Xen-devel] " Boris Ostrovsky
2019-09-08 21:11       ` Igor Druzhinin
2019-09-08 21:11         ` [Xen-devel] " Igor Druzhinin
2019-09-08 23:30         ` Boris Ostrovsky
2019-09-08 23:30           ` [Xen-devel] " Boris Ostrovsky
2019-09-08 23:37           ` Igor Druzhinin
2019-09-08 23:37             ` [Xen-devel] " Igor Druzhinin
2019-09-09 19:19             ` Boris Ostrovsky
2019-09-09 19:19               ` [Xen-devel] " Boris Ostrovsky
2019-09-09 21:48               ` Igor Druzhinin
2019-09-09 21:48                 ` [Xen-devel] " Igor Druzhinin
2019-09-10  1:47                 ` Boris Ostrovsky
2019-09-10  1:47                   ` Boris Ostrovsky
2019-09-10  9:46                   ` Igor Druzhinin
2019-09-10  9:46                     ` Igor Druzhinin
2019-09-10  9:55                     ` Jan Beulich
2019-09-10  9:55                       ` Jan Beulich
2019-09-10 10:08                       ` Igor Druzhinin
2019-09-10 10:08                         ` Igor Druzhinin
2019-09-10 17:48                     ` Boris Ostrovsky
2019-09-10 17:48                       ` Boris Ostrovsky
2019-09-10 20:36                       ` Igor Druzhinin
2019-09-10 20:36                         ` Igor Druzhinin
2019-09-10 21:19                         ` Boris Ostrovsky
2019-09-10 21:19                           ` Boris Ostrovsky
2019-09-11  1:15                           ` Igor Druzhinin
2019-09-11  1:15                             ` Igor Druzhinin
2019-09-11  9:13                             ` Jan Beulich
2019-09-11  9:13                               ` Jan Beulich
2019-09-12 17:33                             ` Boris Ostrovsky [this message]
2019-09-12 17:33                               ` Boris Ostrovsky

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=dd95aa25-bea7-dbc1-fea3-0e21d00d0686@oracle.com \
    --to=boris.ostrovsky@oracle.com \
    --cc=igor.druzhinin@citrix.com \
    --cc=jgross@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.