Re: [PATCH v10 09/28] gpu: nova-core: Hopper/Blackwell: new location for PCI config mirror

Alexandre Courbot
2026-04-20T03:10:21Z

On Sat Apr 18, 2026 at 10:54 AM JST, John Hubbard wrote:
> On 4/17/26 6:46 PM, John Hubbard wrote:
>> On 4/17/26 7:23 AM, Alexandre Courbot wrote:
>>> On Sat Apr 11, 2026 at 11:49 AM JST, John Hubbard wrote:
>> ...
>>>> @@ -24,7 +30,10 @@ pub(crate) struct GspSetSystemInfo {
>>>> impl GspSetSystemInfo {
>>>> /// Returns an in-place initializer for the `GspSetSystemInfo` command.
>>>> #[allow(non_snake_case)]
>>>> - pub(crate) fn init<'a>(dev: &'a pci::Device<device::Bound>) -> impl Init<Self, Error> + 'a {
>>>> + pub(crate) fn init<'a>(
>>>> + dev: &'a pci::Device<device::Bound>,
>>>> + chipset: Chipset,
>>>> + ) -> impl Init<Self, Error> + 'a {
>>>> type InnerGspSystemInfo = bindings::GspSystemInfo;
>>>> let init_inner = try_init!(InnerGspSystemInfo {
>>>> gpuPhysAddr: dev.resource_start(0)?,
>>>> @@ -35,7 +44,14 @@ pub(crate) fn init<'a>(dev: &'a pci::Device<device::Bound>) -> impl Init<Self, E
>>>> // Using TASK_SIZE in r535_gsp_rpc_set_system_info() seems wrong because
>>>> // TASK_SIZE is per-task. That's probably a design issue in GSP-RM though.
>>>> maxUserVa: (1 << 47) - 4096,
>>>> - pciConfigMirrorBase: 0x088000,
>>>> + // Hopper, Blackwell, and later moved the PCI config mirror window to 0x092000.
>>>> + // Older architectures continue to use the legacy window at 0x088000.
>>>> + pciConfigMirrorBase: match chipset.arch() {
>>>> + Architecture::Turing | Architecture::Ampere | Architecture::Ada => 0x088000,
>>>> + Architecture::Hopper
>>>> + | Architecture::BlackwellGB10x
>>>> + | Architecture::BlackwellGB20x => 0x092000,
>>>> + },
>>>
>>> Mmm, similarly to the previous patch, I would prefer to have this behind
>>> a HAL, but I am not quite sure which one would fit. Any idea?
>>
>> This really bothered me, because I distinctly recall very recently putting
>> this behind a HAL! And now I see that I did that in one of the "much later
>> this year: future firmware directions" branches. ha.
>>
>> I went with gpu/hal.rs for this. Yes, it could get large, but let's see
>> how it goes. This is the first item in there. I'll apply it to this series.
>
> Oh wait, I wrote too quickly. That's not quite what I did, but anyway,
> let me take a closer look when I'm not so sleepy. :)

Having this in gpu/hal.rs sounds good to me, fwiw - I don't really see
where else it could be anyway.