Is this a MI interface we want all our MD code to use to cope with similar issues that we're currently coping with in other, ad-hoc ways?

No init at the declaration location.

binary ops require spaces around the op symbol.

What is end ? Is it the address to reserve, or the address right after the reserve ?

oxcf8 is spelled as CONF1_ADDR_PORT, 0x80000000 is CONF1_ENABLE. See x86/include/pci_cfgreg.h.

Both PCI config access method and port accesses are x86 MD. I do not believe that this code belongs to dev/pci.

That said, why did not you used pci_cfregopen() and pci_cfgregread() ? I think that there is no dependencies so normal PCI (not PCIe) config access would work very early.

return (1 * MB);

return (0);

intel_graphics_stolen(void)

Look at style(9) how spaces should be used.

Too many blank lines.

I believe this is highly MD code and interface. Early manipulations with phys_avail[] cannot be anything else.

Do you mean [(i+2)] ?

End is the address right after the reserve. Maybe having the function take base and size as arguments would make things more clear?

Yes. The question is, do we want this function for 32bit as well (I left this open for discussion)? For graphics I can't think of a future we're will need it for i386. If we keep it amd64 only, what would be the correct header for it?

I mean spaces, not braces.

phys_avail[i + 2]

No, it is common to take the range end as the address after the range. But then you have off by one in the check.

We have amd64/include/md_var.h and i386/include/md_var.h. x86_var.h is just a common extract. But I do not see why i386 should be excluded.

Is there a common source file for i386/amd64 that make sense to put phys_avail() in (since the implementations would be the same) or should we duplicate it in machdep.c?

I believe a new source file in x86/x86 is the best.

Look at sys/amd64/amd64/mp_machdep.c:87, where the GiB(x) macro is defined. I like this way much more than manual multiplication by a constant.

See inline comment in the loop iterating over the array.

This blank line is not needed.

return (); there and in all returns down.

Extra blank line, there and in all functions down.

'else' is not needed.

The return is dead.

Well, style recommends avoiding initialization in declaration. There you really provide constants, so perhaps either add const qualifiers to b/s/f names, or use #defines with more telling name, like NB_BUS etc.

Why do you do it this way ? Make the type of data member 'constr struct intel_stolen_ops *' directly.
Also, style: add space before '*', remove space after ')'.

Extra blank line.

Add blank line after '{'.

Don't you need to give the credit to the Intel' rights ? What is the copyright on the equivalent Linux code ?

I remember we discussed that the extern declarations should be moved to sys/amd64/include/md_var.h. I may be mis-remembered, but pcivar.h is a weird place for MD vars.

I am not sure that this makes sense to add, since you do not implemented anything for i386.

The plan was to make a function that could be used for many other things, other than only stolen memory, similar to what Linux does. Hence the generic pointer.

So that data can be a generic pointer for to be used by other early quirk functions.

In Linux this file is a generic early quirks function that handles many different things including stolen memory. There are no copyright names at all.