Re: RE: [RESEND v2 1/2] hw/arm/virt-acpi-build.c: Migrate SPCR creation to common location

2024-01-30 Thread Andrew Jones
On Tue, Jan 30, 2024 at 03:30:13AM +, JeeHeng Sia wrote:
...
> > Sharing code is good, but if we have to parametrize the entire table, then
> > we might as well keep Arm and RISCV separate. Building the table first
> > with this struct, just to have it built again with the build_append API,
> > doesn't make much sense to me. Do Arm and riscv really diverge on all
> > these parameters? If not, then just add the parameters which do diverge
> > build_scpr's arguments.
> It is kind of chicken and egg thing, I would suggest let the arch code to
> fill in the value. It doesn't make sense to change again in the future when
> both riscv and arm realized the parameters were different.
> Can arm confirm that these values wouldn't change in the future?
> > 

We can't be sure that arm nor riscv will change in the future, but we
(arm/riscv/etc. QEMU developers) control the code for both, so I don't see
a problem with only parametrizing what's necessary today and then
extending that, or completely separating, later if necessary. In any case,
I'd rather see the two completely separate from the start, than to see
the structure with all the parameters get added. There's no difference to
me when reading a list of 's->param_name = value' or a list of
build_append_int(table, value, size) /* param_name */. And, given we need
the later eventually anyway, then there's no reason for the former at all.

Thanks,
drew



Re: RE: [RESEND v2 1/2] hw/arm/virt-acpi-build.c: Migrate SPCR creation to common location

2024-01-30 Thread Andrew Jones
On Tue, Jan 30, 2024 at 03:16:34AM +, JeeHeng Sia wrote:
...
> > I think either there should be a comment that this supports only v2 of
> > SPCR spec or it should be able to create SPCR of any version. IMO, I
> > think it is better to add support till v4 (latest). Since consumers like
> > Linux probably doesn't support v4 yet, ARM/RISC-V can continue to create
> > v2 itself for the time being but the generic build_spcr() should be able
> > to create v4 also if the arch requires it.
> A v4 table depends on the updated acpica. I am not aware if there is a
> request from ARM to update to v4. Anyway, RISC-V BRS Spec did mentioned
> on poll-based sbi console. I can check with acpica community if updating
> table to v4 is the go otherwise I would suggest we cont stick to v2 because
> there is no compatible ACPI guest to test the code.
> > 

Generally we want to produce the oldest version which meets the
requirements in order to support the widest set of consumers.

Thanks,
drew