| Message ID | YeB0zGRC/ct8DAzM@work-vm (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ppc pbr403 vmstate | expand |
On Thu, Jan 13, 2022 at 06:51:56PM +0000, Dr. David Alan Gilbert wrote: > Hi, > Is there any easy way of getting a machine where the pbr403 vmstate > would be generated? The condition in pbr403_needed is... return (pvr & 0xffff0000) == 0x00200000; .. which looks to be the PVR for ppc403 models. That makes sense with the section name... but not so much with the fact that it's under cpu/tlb6xx. The 6xx MMU is basically unrelated to the 40x MMU. But it looks like the vmstate_tlbemb might be shared between then, because of bad ideas of the past. But in any case, we already dropped what little 403 support we ever had - there's nothing with that PVR even listed in target/ppc/cpu-models.h. So I think we should just drop it. > Given my vague understanding of vmstate subsection naming, I think > we need: > > diff --git a/target/ppc/machine.c b/target/ppc/machine.c > index 756d8de5d8..e535edb7c4 100644 > --- a/target/ppc/machine.c > +++ b/target/ppc/machine.c > @@ -718,7 +718,7 @@ static bool pbr403_needed(void *opaque) > } > > static const VMStateDescription vmstate_pbr403 = { > - .name = "cpu/pbr403", > + .name = "cpu/tlb6xx/pbr403", > .version_id = 1, > .minimum_version_id = 1, > .needed = pbr403_needed, > > to fit the rule where the name of a subsection is prefixed > by the parent name. (Something a new check I added just triggered). > > Dave >
On 1/14/22 00:41, David Gibson wrote: > On Thu, Jan 13, 2022 at 06:51:56PM +0000, Dr. David Alan Gilbert wrote: >> Hi, >> Is there any easy way of getting a machine where the pbr403 vmstate >> would be generated? > > The condition in pbr403_needed is... > > return (pvr & 0xffff0000) == 0x00200000; > > .. which looks to be the PVR for ppc403 models. That makes sense with > the section name... but not so much with the fact that it's under > cpu/tlb6xx. The 6xx MMU is basically unrelated to the 40x MMU. But > it looks like the vmstate_tlbemb might be shared between then, because > of bad ideas of the past. > > But in any case, we already dropped what little 403 support we ever > had - there's nothing with that PVR even listed in > target/ppc/cpu-models.h. > > So I think we should just drop it. yes. But we can not remove env.pb since this would break migration compatibility, correct ? I will send a patch. Thanks, C.
On Fri, Jan 14, 2022 at 08:07:21AM +0100, Cédric le Goater wrote: > On 1/14/22 00:41, David Gibson wrote: > > On Thu, Jan 13, 2022 at 06:51:56PM +0000, Dr. David Alan Gilbert wrote: > > > Hi, > > > Is there any easy way of getting a machine where the pbr403 vmstate > > > would be generated? > > > > The condition in pbr403_needed is... > > > > return (pvr & 0xffff0000) == 0x00200000; > > > > .. which looks to be the PVR for ppc403 models. That makes sense with > > the section name... but not so much with the fact that it's under > > cpu/tlb6xx. The 6xx MMU is basically unrelated to the 40x MMU. But > > it looks like the vmstate_tlbemb might be shared between then, because > > of bad ideas of the past. > > > > But in any case, we already dropped what little 403 support we ever > > had - there's nothing with that PVR even listed in > > target/ppc/cpu-models.h. > > > > So I think we should just drop it. > > yes. But we can not remove env.pb since this would break migration > compatibility, correct ? Only if it appears in a migration section that's actually emitted by a supported machine type. As far as I can tell the only section that does that is vmstate_pbr403, which we're also dropping so we should be fine. It is also touched in the *super* old cpu_load_old. I suspect we could probably just drop that completely, since I don't think we realistically support migration from a version that old anyway. But even if we don't want to do that right now, we can just replace the reads into env->pb with discarding reads and we'll be fine. We don't implement any cpus that actually used those fields, so we can ignore them in the migration stream.
On 1/17/22 06:52, David Gibson wrote: > On Fri, Jan 14, 2022 at 08:07:21AM +0100, Cédric le Goater wrote: >> On 1/14/22 00:41, David Gibson wrote: >>> On Thu, Jan 13, 2022 at 06:51:56PM +0000, Dr. David Alan Gilbert wrote: >>>> Hi, >>>> Is there any easy way of getting a machine where the pbr403 vmstate >>>> would be generated? >>> >>> The condition in pbr403_needed is... >>> >>> return (pvr & 0xffff0000) == 0x00200000; >>> >>> .. which looks to be the PVR for ppc403 models. That makes sense with >>> the section name... but not so much with the fact that it's under >>> cpu/tlb6xx. The 6xx MMU is basically unrelated to the 40x MMU. But >>> it looks like the vmstate_tlbemb might be shared between then, because >>> of bad ideas of the past. >>> >>> But in any case, we already dropped what little 403 support we ever >>> had - there's nothing with that PVR even listed in >>> target/ppc/cpu-models.h. >>> >>> So I think we should just drop it. >> >> yes. But we can not remove env.pb since this would break migration >> compatibility, correct ? > > Only if it appears in a migration section that's actually emitted by a > supported machine type. As far as I can tell the only section that > does that is vmstate_pbr403, which we're also dropping so we should be > fine. I sent a patch to remove vmstate_pbr403 first. > It is also touched in the *super* old cpu_load_old. I suspect we > could probably just drop that completely, since I don't think we > realistically support migration from a version that old anyway. But > even if we don't want to do that right now, we can just replace the > reads into env->pb with discarding reads and we'll be fine. We don't > implement any cpus that actually used those fields, so we can ignore > them in the migration stream. I will take a look at this also with follow ups. Thanks, C.
* Cédric Le Goater (clg@kaod.org) wrote: > On 1/17/22 06:52, David Gibson wrote: > > On Fri, Jan 14, 2022 at 08:07:21AM +0100, Cédric le Goater wrote: > > > On 1/14/22 00:41, David Gibson wrote: > > > > On Thu, Jan 13, 2022 at 06:51:56PM +0000, Dr. David Alan Gilbert wrote: > > > > > Hi, > > > > > Is there any easy way of getting a machine where the pbr403 vmstate > > > > > would be generated? > > > > > > > > The condition in pbr403_needed is... > > > > > > > > return (pvr & 0xffff0000) == 0x00200000; > > > > > > > > .. which looks to be the PVR for ppc403 models. That makes sense with > > > > the section name... but not so much with the fact that it's under > > > > cpu/tlb6xx. The 6xx MMU is basically unrelated to the 40x MMU. But > > > > it looks like the vmstate_tlbemb might be shared between then, because > > > > of bad ideas of the past. > > > > > > > > But in any case, we already dropped what little 403 support we ever > > > > had - there's nothing with that PVR even listed in > > > > target/ppc/cpu-models.h. > > > > > > > > So I think we should just drop it. > > > > > > yes. But we can not remove env.pb since this would break migration > > > compatibility, correct ? > > > > Only if it appears in a migration section that's actually emitted by a > > supported machine type. As far as I can tell the only section that > > does that is vmstate_pbr403, which we're also dropping so we should be > > fine. > > I sent a patch to remove vmstate_pbr403 first. Thanks! Dave > > It is also touched in the *super* old cpu_load_old. I suspect we > > could probably just drop that completely, since I don't think we > > realistically support migration from a version that old anyway. But > > even if we don't want to do that right now, we can just replace the > > reads into env->pb with discarding reads and we'll be fine. We don't > > implement any cpus that actually used those fields, so we can ignore > > them in the migration stream. > > I will take a look at this also with follow ups. > > Thanks, > > C. >
On Mon, 17 Jan 2022 at 05:52, David Gibson <david@gibson.dropbear.id.au> wrote: > It is also touched in the *super* old cpu_load_old. I suspect we > could probably just drop that completely, since I don't think we > realistically support migration from a version that old anyway. This would be a nice thing to do, because the PPC CPU is the only remaining in-tree user of the .load_state_old migration hook, so if we declared that we don't support migrating from those old versions of QEMU (1.5 and earlier, I think) then we could delete the .load_state_old and .minimum_version_id_old handling completely. -- PMM
On 1/17/22 21:40, Peter Maydell wrote: > On Mon, 17 Jan 2022 at 05:52, David Gibson <david@gibson.dropbear.id.au> wrote: >> It is also touched in the *super* old cpu_load_old. I suspect we >> could probably just drop that completely, since I don't think we >> realistically support migration from a version that old anyway. > > This would be a nice thing to do, because the PPC CPU is the > only remaining in-tree user of the .load_state_old migration > hook, so if we declared that we don't support migrating from > those old versions of QEMU (1.5 and earlier, I think) then we > could delete the .load_state_old and .minimum_version_id_old > handling completely. yes. It should be fine for PPC. Thanks, C.
diff --git a/target/ppc/machine.c b/target/ppc/machine.c index 756d8de5d8..e535edb7c4 100644 --- a/target/ppc/machine.c +++ b/target/ppc/machine.c @@ -718,7 +718,7 @@ static bool pbr403_needed(void *opaque) } static const VMStateDescription vmstate_pbr403 = { - .name = "cpu/pbr403", + .name = "cpu/tlb6xx/pbr403", .version_id = 1, .minimum_version_id = 1, .needed = pbr403_needed,