Attention is currently required from: Arthur Heymans, Jérémy Compostella.
7 comments:
Patchset:
Mostly duplication concerns...
Is it too late to bikeshed the new API? I feel silly because I […]
Looking ahead, it doesn't seem to matter if we'd change it now or later.
File src/cpu/amd/smm/amd64_save_state.c:
Patch Set #4, Line 10: - sizeof(amd64_smm_state_save_area_t));
Shouldn't this be covered by setting `.save_state_size` to the correct value?
Or can't we probe that before we are in SMM? But then, why do we have a callback
in the mp init to decide it?
Either way, it would seem better to make smm_get_save_state() return the correct
pointer right away.
Patch Set #4, Line 118: 0x00030064,
Where to find these? Please mention in the commit message.
File src/cpu/intel/smm/em64t100_save_state.c:
static int em64t100_get_set(const enum get_set op_type, const enum cpu_reg reg,
const int node, void *in_out, const uint8_t length)
{
void *reg_base = em64t100_get_reg_base(reg, node);
if (!reg_base)
return -1;
switch (length) {
case 1:
case 2:
case 4:
case 8:
switch (op_type) {
case GET:
memcpy(in_out, reg_base, length);
return 0;
case SET:
memcpy(reg_base, in_out, length);
return 0;
}
}
return -1;
}
static int em64t100_get_reg(const enum cpu_reg reg, const int node, void *out,
const uint8_t length)
{
return em64t100_get_set(GET, reg, node, out, length);
}
static int em64t100_set_reg(const enum cpu_reg reg, const int node, void *in,
const uint8_t length)
{
return em64t100_get_set(SET, reg, node, in, length);
}
So these look like generic code. If we want to keep the API. The save_state_ops
cut could be made at get_reg_base() to avoid the duplication.
File src/cpu/intel/smm/em64t101_save_state.c:
Could be made generic by taking `io_misc_info` & `rax` as parameters.
File src/cpu/x86/smm/legacy_save_state.c:
This looks like it would go wrong 1 out of 512 times on a dual core / socket system.
Is there no way to detect the AMPC or did just nobody look into it?
To view, visit change 83064. To unsubscribe, or for help writing mail filters, visit settings.